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.
Differential D112286
Generalize DWARF tests that expect DW_FORM_strp Jake-Egan on Oct 21 2021, 5:59 PM. Authored by
Details 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. 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.
|
I'd drop the "= " here (& elsewhere in the patch), like it's done elsewhere