The modified tests were failing on AIX because DWARF on AIX uses inline strings by default, but the tests check for DW_FORM_strp.
This patch removes DW_FORM_strp so both forms will pass the check.
Paths
| Differential D112286
Generalize DWARF tests that expect DW_FORM_strp ClosedPublic Authored by Jake-Egan on Oct 21 2021, 5:59 PM.
Details Summary The modified tests were failing on AIX because DWARF on AIX uses inline strings by default, but the tests check for DW_FORM_strp. This patch removes DW_FORM_strp so both forms will pass the check.
Diff Detail
Event TimelineComment Actions Maybe we could split this patch into two: The second one looks reasonable to me. Comment Actions
^ agreed. Would be good to understand why llvm-dwarfdump is doing anything different in terms of whitespace on AIX... I can't think of any reason that should be the case. Comment Actions Here is an example for the differing output with file llvm/test/DebugInfo/Generic/varargs.ll On Linux: DW_AT_name [DW_FORM_strp]^I( .debug_str[0x00000065] = "a")$ On AIX: DW_AT_name [DW_FORM_string]^I("a")$ I will create a new patch for the white space issue and convert this patch to add the -dwarf-inlined-strings=Disable option. Jake-Egan retitled this revision from Adapt DWARF tests for AIX to Add -dwarf-inlined-strings=Disable option to DWARF tests that expect DW_FORM_strp.Oct 24 2021, 4:29 PM This revision is now accepted and ready to land.Oct 24 2021, 8:06 PM Comment Actions I think probably generalizing the tests to not care about the string encodings is probably the right thing, rather than forcing indirect strings? I think these tests are overly specific - they don't look like they should care about the string encoding (& Probably an artifact of older techniques in writing these debug info tests - improvements to the dumper to print strings inline, skip forms, etc) Comment Actions
Thanks for showing the difference - I think these can probably most often be tested with CHECK: DW_AT_name {{.*}}"a" is probably fine - or if the tests don't need -v output (doesn't dump forms, etc), it'll probably be simpler, may even be exactly the same output between strp and str encodings. Comment Actions
I agree generalizing the test is probably the better option if DW_FORM_strp isn't necessary for the purpose of the test. @shchenz I will update the patch if you have no objection. Comment Actions
Sounds good to me Jake-Egan retitled this revision from Add -dwarf-inlined-strings=Disable option to DWARF tests that expect DW_FORM_strp to Generalize DWARF tests that expect DW_FORM_strp.Oct 25 2021, 8:23 PM
Closed by commit rG9feb46137560: Generalize DWARF tests that expect DW_FORM_strp (authored by Jake-Egan). · Explain WhyOct 26 2021, 8:43 PM This revision was automatically updated to reflect the committed changes.
Revision Contents
Diff 381446 llvm/test/DebugInfo/Generic/2010-04-06-NestedFnDbgInfo.ll
llvm/test/DebugInfo/Generic/PR20038.ll
llvm/test/DebugInfo/Generic/constant-pointers.ll
llvm/test/DebugInfo/Generic/containing-type-extension.ll
llvm/test/DebugInfo/Generic/cross-cu-inlining.ll
llvm/test/DebugInfo/Generic/cross-cu-linkonce-distinct.ll
llvm/test/DebugInfo/Generic/cross-cu-linkonce.ll
llvm/test/DebugInfo/Generic/dead-argument-order.ll
llvm/test/DebugInfo/Generic/debug-label-inline.ll
llvm/test/DebugInfo/Generic/debug-label.ll
llvm/test/DebugInfo/Generic/disubrange_vla.ll
llvm/test/DebugInfo/Generic/enum-types.ll
llvm/test/DebugInfo/Generic/enum.ll
llvm/test/DebugInfo/Generic/fortran-subprogram-attr.ll
llvm/test/DebugInfo/Generic/gmlt_profiling.ll
llvm/test/DebugInfo/Generic/imported-name-inlined.ll
llvm/test/DebugInfo/Generic/incorrect-variable-debugloc.ll
llvm/test/DebugInfo/Generic/linkage-name-abstract.ll
llvm/test/DebugInfo/Generic/mainsubprogram.ll
llvm/test/DebugInfo/Generic/member-order.ll
llvm/test/DebugInfo/Generic/missing-abstract-variable.ll
llvm/test/DebugInfo/Generic/namespace.ll
llvm/test/DebugInfo/Generic/namespace_function_definition.ll
llvm/test/DebugInfo/Generic/namespace_inline_function_definition.ll
llvm/test/DebugInfo/Generic/recursive_inlining.ll
llvm/test/DebugInfo/Generic/restrict.ll
llvm/test/DebugInfo/Generic/thrownTypes.ll
llvm/test/DebugInfo/Generic/tu-composite.ll
llvm/test/DebugInfo/Generic/varargs.ll
llvm/test/DebugInfo/Inputs/gmlt.ll
llvm/test/Linker/Inputs/type-unique-simple2-a.ll
llvm/test/Linker/type-unique-odr-a.ll
llvm/test/Linker/type-unique-simple-a.ll
llvm/test/Linker/type-unique-simple2-a.ll
llvm/test/Linker/type-unique-type-array-a.ll
|
I'd drop the "= " here (& elsewhere in the patch), like it's done elsewhere