dblaikie added inline comments.
================ Comment at: lldb/test/API/functionalities/tail_call_frames/disambiguate_paths_to_common_sink/main.cpp:8 + // FROM-FUNC1-NEXT: func1 + // FROM-FUNC1-SAME: [artificial] + // FROM-FUNC1-NEXT: main ---------------- vsk wrote: > dblaikie wrote: > > vsk wrote: > > > labath wrote: > > > > labath wrote: > > > > > dblaikie wrote: > > > > > > vsk wrote: > > > > > > > labath wrote: > > > > > > > > vsk wrote: > > > > > > > > > Are these test updates necessary because lldb doesn't print > > > > > > > > > '[opt]' and '[artificial]' next to frame descriptions in a > > > > > > > > > consistent way across platforms? Or is it just that you don't > > > > > > > > > think matching '[opt]' is relevant to the test? > > > > > > > > Right, I wanted to mention that as it's not very obvious, but I > > > > > > > > forgot... > > > > > > > > > > > > > > > > The `[opt]` thingy is not printed at all with -ggdb because the > > > > > > > > attribute we get this information from -- DW_AT_APPLE_optimized > > > > > > > > -- is only emitted for -glldb. The optimization flag did not > > > > > > > > seem very relevant for these tests (I mean, technically the > > > > > > > > compiler could emit call site attributes even in non-optimized > > > > > > > > mode) so instead of forking the expectations I chose to simply > > > > > > > > remove it. > > > > > > > Sounds good. > > > > > > As an aside, now that lldb understands these attributes - perhaps > > > > > > we should emit them under -glldb as well as -ggdb? (@aprantl might > > > > > > be interested in making that call) > > > > > FWIW, I think that would be great as it would reduce the effects of > > > > > the debugger tuning argument, making the compiler output more > > > > > "portable". > > > > > > > > > > Though, we may want to wait with that until I look at the -1 issue. I > > > > > believe that the way this is implemented now means we will end up > > > > > pointing to the middle of a call instruction in an artificial frame, > > > > > which would be a slight regression. It's not the end of the world, > > > > > but I believe we can do something slightly better. > > > > Ok, I take that back. The instruction pointer handling is not terribly > > > > consistent right now anyway: > > > > ``` > > > > (lldb) up > > > > frame #1: 0x0000000000401210 a.out`func12(...) > > > > (lldb) register read rip > > > > rip = 0x0000000000401300 > > > > ``` > > > > > > > > So, I wouldn't worry too much about preserving behavior here. > > > I don't see any concrete benefit to supporting -ggdb on Darwin. Actually, > > > changing llvm to emit the GNU opcodes on Darwin seems bad to me, as it > > > could force Darwin tools authors to support two sets of call-site related > > > opcodes. > > Not sure what it would mean to "support -ggdb on Darwin" - it's supported > > in that some peolpe might be using gdb on Darwin & compile that way. But I > > meant emitting these when targeting lldb - which someone might be doing on > > any platform & they might still want to use DWARFv4 for whatever reason - > > or does DWARFv4 + -glldb already use the DWARFv5 call site opcodes? If so, > > then, sure, that sounds OK to me & I don't suppose there's much reason to > > emit the GNU ones instead. If DWARFv4 + -glldb doesn't emit any call site > > info, it seems like an improvement to emit the GNU extension (consumers > > should always be written to ignore tags/attributes they don't know - and if > > so they'll be no worse off than if the call site info hadn't been emitted) > > or the v5 attributes maybe (though I don't know that that's quite as valid > > - I guess a consumer could rightly reject tags/attributes that aren't in > > the extension number space, or in the standard number space for the version > > being parsed) > Yep, DWARFv4 + -glldb emits DWARFv5 call site opcodes -- and I should have > written (as I meant), 'there's no need to change this to behave like -ggdb > under -gdwarf-4 mode on Darwin'. It should (remain!) possible to use -ggdb on > Darwin. Ah, fair enough - marginally questionable emitting future standard tags/attributes in previous versions - but I'm not too fussed so long as it's not new forms. As that's the direction chosen, yeah, no point emitting the DWARFv4 gdb extension tags/attributes under -glldb. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80519/new/ https://reviews.llvm.org/D80519 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
