firmiana402 wrote: > Based on my reading of `2.19 Static and Dynamic Values of Attributes` of the > DWARFv5 spec, it sounds like we should be using this approach in many more > places than just array bounds. Which makes me wonder: > > 1. do we already have this functionality elsewhere in LLDB and just need > to use it for array bounds? > > 2. have we mostly been getting away with interpreting attributes as > constants because that's how C++ debug-info generally looks like? IIUC other > languages (like Ada) have much more dynamic debug-info relying on DWARF > expression evaluation. Out of curiosity, what compiler/language is emitting > the array bounds you're seeing? > > 3. we should think about placing this somewhere more common so the other > DWARF parser components can make use of it (though that's out of scope of > this PR) > > > Regarding testing, it'd be nice we could add unit-tests for the static array > bounds (if we don't already have that...just grepping for `DW_AT_upper_bound` > in the `lldb/unittests/` directory should tell us that). Looks like we still > need the shell test to cover the execution context case
Thanks for the review! 1. Good point on the naming. Would `EvaluateArrayPropertyAsUnsigned` read better? The intent isn't that the encoded form must be unsigned, but that array properties such as bounds/count/stride are evaluated into an unsigned result. I'll also add a comment above the helper to make the two supported cases explicit: directly encoded constants, and expression blocks that must be evaluated in an execution context. 2. I also agree that DWARF v5 suggests this problem is broader than array bounds. I looked for an existing LLDB helper to reuse here, but I couldn't find one that directly models "evaluate this attribute value as a dynamic DWARF attribute value". There are several expression-evaluation paths, but each is tied to a specific semantics. The closest match is `ExtractDataMemberLocation` (in [DWARFASTParserClang.cpp](https://github.com/llvm/llvm-project/blob/main/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp)), but it's specific to data-member offsets and runs during static type parsing without an `ExecutionContext`, so it can't handle frame-dependent expressions such as `DW_OP_fbreg`. 3. The case I'm currently seeing is C VLA debug info emitted by GCC (see https://godbolt.org/z/zjq9PP16f) ``` ... 0x000000c6: DW_TAG_array_type DW_AT_type (0x00000058 "int") 0x000000cb: DW_TAG_subrange_type DW_AT_type (0x0000002e "long unsigned int") DW_AT_upper_bound (DW_OP_fbreg -24, DW_OP_deref) ... ``` And I know that Fortran relies on these dynamic forms far more extensively. 4. Agreed that placing this into a more common DWARF parser component could make sense, especially if we find other attributes affected by the same issue. For this PR, I'd prefer to keep the fix local to array properties. 5. Agreed on the logging and testing suggestions. Does that direction sound reasonable to you? https://github.com/llvm/llvm-project/pull/204119 _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
