[BOLT][DWARF] updateDWARFObjectAddressRanges: nullify low pc

[yota9]

Summary

In case the case the DW_AT_ranges tag already exists for the object the
low pc values won't be updated and will be incorrect in
after-bolt binaries.

Due to the large test binary the test is prepared separately in
https://github.com/rafaelauler/bolt-tests/pull/8

Vladislav Khmelevsky,
Advanced Software Technology Lab, Huawei

[ayermolo]

How hard will it be to add a small repro llvm test for this?
Something similar to bolt/test/X86/debug-fission-single.s
Final push. Sorry it's taking so long.

[yota9]

@ayermolo Sorry I'm not quite sure how to manipulate the DWARF in C for example, so it will generate such a combination of tags and attributes. Except of a hexdump and changing them by hands... I would glad to hear the better idea :)

[ayermolo]

In D117216#3242835, @yota9 wrote:

@ayermolo Sorry I'm not quite sure how to manipulate the DWARF in C for example, so it will generate such a combination of tags and attributes. Except of a hexdump and changing them by hands... I would glad to hear the better idea :)

Previously when I created unit tests I either used YAML2OBJ or compiling small "hello world" program and stripping resulting assembly as much as possible by hand.
There is also a question of BOLT hitting that code path. For this year I was thinking of adding "test mode" for debug info where we can kind of skip all other steps and "force" debug update path to do what we want. It's just an idea for now.
At least for this, I don't think it should be an issue.

[ayermolo]

Also can you add [BOLT][DWARF] tags to the title. Sorry forgot to mention in other diff.

[yota9]

Hello @ayermolo! It's to hard to decrease the golang binary size. So I've devided to hexedit the C hello-word dwarf to get the fixed cases of this and previous dward reviws :)

[ayermolo]

In D117216#3249532, @yota9 wrote:

Hello @ayermolo! It's to hard to decrease the golang binary size. So I've devided to hexedit the C hello-word dwarf to get the fixed cases of this and previous dward reviws :)

Thanks for adding the test.
I wonder if it can be reduced further considering we don't actually care if binary is executable, just that bolt processes it and debug info is changed.

[yota9]

@ayermolo Sure, this is the best I can get fast. I wish there will be automated process. It is great that even the beginning file was small enough. I won't be able to reduce golang test by hands in a reasonable time..

[ayermolo]

Thanks for working on this.

[yota9]

remove couple more sections

[yota9]

@ayermolo Thank you too! :)
Off topic: Sorry, did you managed to locate the problem with dwarfdump?

[ayermolo]

In D117216#3252117, @yota9 wrote:

@ayermolo Thank you too! :)
Off topic: Sorry, did you managed to locate the problem with dwarfdump?

It's on my todo list for this week to see what bolt is messing up. :)

[Amir]

In D117216#3252061, @yota9 wrote:

@ayermolo Sure, this is the best I can get fast. I wish there will be automated process. It is great that even the beginning file was small enough. I won't be able to reduce golang test by hands in a reasonable time..

Sorry for a late comment, but the following might be helpful:

• If golang compiler lowers to LLVM IR, it should be possible to dump it and reduce it using llvm-reduce,
• If not, but the compiler can emit an assembly (with all DWARF sections), it might be possible to reduce it using creduce.
• Otherwise, we'd need to work on extending BOLT's asm-dump to also dump DWARF sections, and reduce the resulting assembly.

Also, the commit title is out of sync with diff's title. Please double-check the commit before the push, and sync from phabricator as necessary.

[yota9]

@Amir Thank you for your answer! I will check these tools.

Also, the commit title is out of sync with diff's title. Please double-check the commit before the push, and sync from phabricator as necessary.

You are right, it is my bad :( I forgot to "arc patch" before the push, I will try to be more careful next time :)

[yota9]

Thanks again @Amir

If golang compiler lowers to LLVM IR

No, it is stand-alone compiler. The LLVM has it's own implementation but it is incompatible with the default gc and I didn't even try to support it

If not, but the compiler can emit an assembly (with all DWARF sections), it might be possible to reduce it using creduce.

Do you mean to compile to object files, using creduce find the needed lines and emit it with linker? I'm not sure if it is possible. The golang compile may produce the archive files, although I didn't try it and not sure what kind of output it has. AFAIU it is very problematic since it is not intended to do such things, it is mostly intended to produce the ready executable files (even the SOs are used very very rarely in golang world) and it is very limited compared to the standard C compiler. But I don't exclude that using some hacks and tricks it is possible.

Otherwise, we'd need to work on extending BOLT's asm-dump to also dump DWARF sections, and reduce the resulting assembly.

Probably it is the best solution here, although I hope we won't need it :))

[Amir]

In D117216#3252320, @yota9 wrote:

Thanks again @Amir

If golang compiler lowers to LLVM IR

No, it is stand-alone compiler. The LLVM has it's own implementation but it is incompatible with the default gc and I didn't even try to support it

Got it.

If not, but the compiler can emit an assembly (with all DWARF sections), it might be possible to reduce it using creduce.

Do you mean to compile to object files, using creduce find the needed lines and emit it with linker? I'm not sure if it is possible. The golang compile may produce the archive files, although I didn't try it and not sure what kind of output it has. AFAIU it is very problematic since it is not intended to do such things, it is mostly intended to produce the ready executable files (even the SOs are used very very rarely in golang world) and it is very limited compared to the standard C compiler. But I don't exclude that using some hacks and tricks it is possible.

No, I mean to dump assembly. I did a quick google search and the following options are available:

• go tool compile -S file.go > file.s
• gccgo -S test.go

Then inspect the assembly, strip out the irrelevant parts while still preserving the integrity of DWARF data.

[yota9]

Oh I see, thanks. But I don't think it will work great. The test I've used is like a few lines of code, most of the code is coming from the golang library, so such a compile method won't help anyway :)

bolt/lib/Rewrite/DWARFRewriter.cpp

if (RangesBase) {
     if (RangesBaseAttrInfo) {
       DebugInfoPatcher.addLE32Patch(RangesBaseAttrInfo->Offset,
                                     static_cast<uint32_t>(*RangesBase),
                                     RangesBaseAttrInfo->Size);
       RangesBase = None;
     }
   }
 
+  Optional<AttrInfo> LowPCAttrInfo =
+      findAttributeInfo(DIE, dwarf::DW_AT_low_pc);
   if (AbbreviationDecl->findAttributeIndex(dwarf::DW_AT_ranges)) {
     // Case 1: The object was already non-contiguous and had DW_AT_ranges.
     // In this case we simply need to update the value of DW_AT_ranges
     // and introduce DW_AT_GNU_ranges_base if required.
     Optional<AttrInfo> AttrVal = findAttributeInfo(DIE, dwarf::DW_AT_ranges);
-
     std::lock_guard<std::mutex> Lock(DebugInfoPatcherMutex);
     DebugInfoPatcher.addLE32Patch(
         AttrVal->Offset, DebugRangesOffset - DebugInfoPatcher.getRangeBase(),
         AttrVal->Size);
-    if (!RangesBase)
+
+    if (!RangesBase) {
+      if (LowPCAttrInfo &&
+          LowPCAttrInfo->V.getForm() != dwarf::DW_FORM_GNU_addr_index &&
+          LowPCAttrInfo->V.getForm() != dwarf::DW_FORM_addrx)
+        DebugInfoPatcher.addLE64Patch(LowPCAttrInfo->Offset, 0);
       return;
+    }
 
     // Convert DW_AT_low_pc into DW_AT_GNU_ranges_base.
-    Optional<AttrInfo> LowPCAttrInfo =
-        findAttributeInfo(DIE, dwarf::DW_AT_low_pc);
     if (!LowPCAttrInfo) {
       errs() << "BOLT-ERROR: skeleton CU at 0x"
              << Twine::utohexstr(DIE.getOffset())
              << " does not have DW_AT_GNU_ranges_base or DW_AT_low_pc to"
                 " convert to update ranges base\n";
       return;
     }
 
     AbbrevWriter.addAttributePatch(
         *DIE.getDwarfUnit(), AbbreviationDecl, dwarf::DW_AT_low_pc,
         dwarf::DW_AT_GNU_ranges_base, dwarf::DW_FORM_indirect);
     DebugInfoPatcher.addUDataPatch(LowPCAttrInfo->Offset, dwarf::DW_FORM_udata,
                                    1);
     DebugInfoPatcher.addUDataPatch(LowPCAttrInfo->Offset + 1, *RangesBase, 7);
 
     return;
   }
 
   // Case 2: The object has both DW_AT_low_pc and DW_AT_high_pc emitted back
   // to back. Replace with new attributes and patch the DIE.
-  if (AbbreviationDecl->findAttributeIndex(dwarf::DW_AT_low_pc) &&
-      AbbreviationDecl->findAttributeIndex(dwarf::DW_AT_high_pc)) {
+  Optional<AttrInfo> HighPCAttrInfo =
+      findAttributeInfo(DIE, dwarf::DW_AT_high_pc);
+  if (LowPCAttrInfo && HighPCAttrInfo) {
     convertToRangesPatchAbbrev(*DIE.getDwarfUnit(), AbbreviationDecl,
                                AbbrevWriter, RangesBase);
     convertToRangesPatchDebugInfo(DIE, DebugRangesOffset, DebugInfoPatcher,
                                   RangesBase);
   } else {
     if (opts::Verbosity >= 1)
       errs() << "BOLT-ERROR: cannot update ranges for DIE at offset 0x"
              << Twine::utohexstr(DIE.getOffset()) << '\n';

bolt/test/AArch64/Inputs/go_dwarf.yaml

+--- !ELF
+FileHeader:
+  Class:           ELFCLASS64
+  Data:            ELFDATA2LSB
+  Type:            ET_DYN
+  Machine:         EM_AARCH64
+  Entry:           0x684
+ProgramHeaders:
+  - Type:            PT_PHDR
+    Flags:           [ PF_R ]
+    VAddr:           0x200
+    Align:           0x8
+  - Type:            PT_INTERP
+    Flags:           [ PF_R ]
+    FirstSec:        .plt
+    LastSec:         .plt
+    VAddr:           0x238
+  - Type:            PT_LOAD
+    Flags:           [ PF_X, PF_R ]
+    FirstSec:        .plt
+    LastSec:         .text
+    Align:           0x10000
+    VAddr:           0x5e8
+Sections:
+  - Name:            .plt
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC, SHF_EXECINSTR ]
+    Address:         0x5F0
+    AddressAlign:    0x10
+    EntSize:         0x10
+    Content:         F07BBFA99000009011CA47F910423E9120021FD61F2003D51F2003D51F2003D59000009011CE47F910623E9120021FD69000009011D247F910823E9120021FD69000009011D647F910A23E9120021FD69000009011DA47F910C23E9120021FD69000009011DE47F910E23E9120021FD6
+  - Name:            .text
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC, SHF_EXECINSTR ]
+    Address:         0x660
+    AddressAlign:    0x8
+    Content:         FD7BBFA9A1D6811200000090FD03009100C02091F7FFFF9700008052FD7BC1A8C0035FD61D0080D21E0080D2E50300AAE10340F9E2230091E60300918000009000F847F98300009063F447F98400009084E447F9DBFFFF97E2FFFF978000009000F047F9400000B4DAFFFF17C0035FD6800000B000400091810000B0214000913F0000EBC00000548100009021E847F9610000B4F00301AA00021FD6C0035FD6800000B000400091810000B021400091210000CB22FC7FD3410C818BFF0781EB21FC4193C00000548200009042FC47F9620000B4F00302AA00021FD6C0035FD6FD7BBEA9FD030091F30B00F9930000B060424039400100358000009000EC47F9800000B4800000B0000440F9A9FFFF97D8FFFF972000805260420039F30B40F9FD7BC2A8C0035FD6DEFFFF171F2003D5FD7BBCA9FD030091F35301A99400009094223691F55B02A995000090B5023691940215CBF603002AF76303A9F70301AAF80302AA83FFFF97FF0F94EB6001005494FE4393130080D2A37A73F8E20318AA73060091E10317AAE003162A60003FD69F0213EB21FFFF54F35341A9F55B42A9F76343A9FD7BC4A8C0035FD61F2003D5C0035FD6
+  - Name:            .debug_info
+    Type:            SHT_PROGBITS
+    AddressAlign:    0x1
+    Content:         B100000004000000000008011F0000000C000000000C00000000000000600600000000000000000000029E0000000101059000000060060000000000008406000000000000019C9000000003610001020990000000CA6204050000000200900000006D00000005970000000600077806000000000000A900000008015009033008000000000000080151030B4AF10000090405696E74000A08A40000000B0108A30000000C9D0000000D0500000005000000020000
+  - Name:            .debug_abbrev
+    Type:            SHT_PROGBITS
+    AddressAlign:    0x1
+    Content:         011101250E130B030E1B0E5517110110170000022E013F19030E3A0B3B0B390B49131101120140189742190113000003340003083A0B3B0B390B49131C0D0000042E013F19030E3A0B3B0B271949133C19011300000505004913000006180000000789820101110131130000088A820100021891421800000924000B0B3E0B030800000A0F000B0B491300000B24000B0B3E0B030E00000C2600491300000D2E003F193C196E0E030E3A0B3B0B000000
+  - Name:            .debug_line
+    Type:            SHT_PROGBITS
+    AddressAlign:    0x1
+    Content:         5C0000000300290000000401FB0E0D0001010101000000010000010065782E63010000003C6275696C742D696E3E0000000000050C00090260060000000000000105051313050C0610050522050C2C050522050C1E0505140501210203000101
+Symbols:
+  - Name:            main
+    Type:            STT_FUNC
+    Section:         .text
+    Binding:         STB_GLOBAL
+    Value:           0x660
+    Size:            0x24
+DWARF:
+  debug_str:
+    - te.c
+    - printf
+    - '/home/test/test/te'
+    - 'GNU C17 9.3.0 -mlittle-endian -mabi=lp64 -g -O3 -fasynchronous-unwind-tables -fstack-protector-strong -fstack-clash-protection'
+    - main
+    - char
+  debug_aranges:
+    - Length:          0x2C
+      Version:         2
+      CuOffset:        0x0
+      AddressSize:     0x8
+      Descriptors:
+        - Address:         0x660
+          Length:          0x24
+  debug_ranges:
+    - Offset:          0x0
+      AddrSize:        0x8
+      Entries:
+        - LowOffset:       0x0
+          HighOffset:      0x24
+...

bolt/test/AArch64/go_dwarf.test

+# Check that the llvm-bolt update the dwarf information correctly in case:
+#  - DW_AT_low_pc is nullified in case the DW_AT_ranges tag already exists.
+#  - DW_AT_high_pc is in the form of DW_FORM_addr.
+
+RUN: yaml2obj %p/Inputs/go_dwarf.yaml &> %t.exe
+RUN: llvm-bolt %t.exe -o %t.bolt -update-debug-sections
+
+# Check the original binary values.
+RUN: llvm-dwarfdump -debug-info %t.exe |& \
+RUN:   FileCheck %s -check-prefix=CHECKORIG
+
+CHECKORIG:         DW_TAG_compile_unit
+CHECKORIG-NEXT:        DW_AT_producer
+CHECKORIG-NEXT:        DW_AT_language
+CHECKORIG-NEXT:        DW_AT_name
+CHECKORIG-NEXT:        DW_AT_comp_dir
+CHECKORIG-NEXT:        DW_AT_ranges      (0x00000000
+CHECKORIG-NEXT:           [0x0000000000000660, 0x0000000000000684))
+CHECKORIG-NEXT:        DW_AT_low_pc      (0x0000000000000660)
+
+CHECKORIG:         DW_TAG_subprogram
+CHECKORIG-NEXT:        DW_AT_external  (true)
+CHECKORIG-NEXT:        DW_AT_name      ("main")
+CHECKORIG-NEXT:        DW_AT_decl_file
+CHECKORIG-NEXT:        DW_AT_decl_line (1)
+CHECKORIG-NEXT:        DW_AT_decl_column       (0x05)
+CHECKORIG-NEXT:        DW_AT_type      (0x00000090 "int")
+CHECKORIG-NEXT:        DW_AT_low_pc    (0x0000000000000660)
+CHECKORIG-NEXT:        DW_AT_high_pc   (0x0000000000000684)
+
+
+# Check the bolted binary.
+RUN: llvm-dwarfdump -debug-info %t.bolt |& FileCheck %s
+
+CHECK:         DW_TAG_compile_unit
+CHECK-NEXT:        DW_AT_producer
+CHECK-NEXT:        DW_AT_language
+CHECK-NEXT:        DW_AT_name
+CHECK-NEXT:        DW_AT_comp_dir
+CHECK-NEXT:        DW_AT_ranges      (0x00000010
+CHECK-NEXT:           [0x0000000000000660, 0x0000000000000684))
+CHECK-NEXT:        DW_AT_low_pc      (0x0000000000000000)
+CHECK-NEXT:        DW_AT_stmt_list   (0x00000000)
+
+CHECK:         DW_TAG_subprogram
+CHECK-NEXT:        DW_AT_external  (true)
+CHECK-NEXT:        DW_AT_name      ("main")
+CHECK-NEXT:        DW_AT_decl_file
+CHECK-NEXT:        DW_AT_decl_line (1)
+CHECK-NEXT:        DW_AT_decl_column       (0x05)
+CHECK-NEXT:        DW_AT_type      (0x00000090 "int")
+CHECK-NEXT:        DW_AT_low_pc    (0x0000000000000660)
+CHECK-NEXT:        DW_AT_high_pc   (0x0000000000000684)