[BOLT][DWARF] Fix how DW_AT_high_pc [DW_FORM_udata] is handled

[ayermolo]

Summary

We were not handling correctly conversion from DW_AT_high_pc into DW_AT_ranges,
when size of DW_AT_high_pc is not 4/8 bytes.

[ayermolo]

small fix

[maksfb]

bolt/include/bolt/Core/DebugData.h

540

Could you add a description of the patch kind in a comment?

546

I don't see the need to redefine getKind().

[ayermolo]

address comments

[maksfb]

nit: drop the trailing period from the title.

bolt/include/bolt/Core/DebugData.h

 };
 
 class DebugInfoBinaryPatcher : public SimpleBinaryPatcher {
 public:
   enum class DebugPatchKind {
     PatchBaseClass,
     PatchValue32,
     PatchValue64to32,
+    PatchValue32GenericSize,
     PatchValue64,
     PatchValueVariable,
     ReferencePatchValue,
     DWARFUnitOffsetBaseLabel,
     DestinationReferenceLabel,
     NewDebugEntry
   };
 
struct DebugPatch32 : public Patch {
     DebugPatchKind getKind() const { return Kind; }
 
     static bool classof(const Patch *Writer) {
       return Writer->getKind() == DebugPatchKind::PatchValue32;
     }
     uint32_t Value;
   };
 
+  struct DebugPatch32GenericSize : public Patch {
+    DebugPatch32GenericSize(uint32_t O, uint32_t V, uint32_t OVS)
+        : Patch(O, DebugPatchKind::PatchValue32GenericSize) {
+      Value = V;
+      OldValueSize = OVS;
+    }
+    DebugPatchKind getKind() const { return Kind; }
+
+    static bool classof(const Patch *Writer) {
+      return Writer->getKind() == DebugPatchKind::PatchValue32GenericSize;
+    }
+    uint32_t Value;
+    uint32_t OldValueSize;
+  };
+
   struct DebugPatch64 : public Patch {
     DebugPatch64(uint32_t O, uint64_t V)
         : Patch(O, DebugPatchKind::PatchValue64) {
       Value = V;
     }
 
     static bool classof(const Patch *Writer) {
       return Writer->getKind() == DebugPatchKind::PatchValue64;
     }
     uint64_t Value;
   };
 
   struct DebugPatchVariableSize : public Patch {
-    DebugPatchVariableSize(uint32_t O, uint32_t OVS, uint32_t V)
+    DebugPatchVariableSize(uint32_t O, uint32_t V, uint32_t OVS)
         : Patch(O, DebugPatchKind::PatchValueVariable) {
-      OldValueSize = OVS;
       Value = V;
+      OldValueSize = OVS;
     }
 
     static bool classof(const Patch *Writer) {
       return Writer->getKind() == DebugPatchKind::PatchValueVariable;
     }
-    uint32_t OldValueSize;
     uint32_t Value;
+    uint32_t OldValueSize;
   };
 
   struct DebugPatchReference : public Patch {
     struct Data {
       uint32_t OldValueSize : 4;
       uint32_t DirectRelative : 1;
       uint32_t IndirectRelative : 1;
       uint32_t DirectAbsolute : 1;
void operator()(Patch *P) const {
         llvm_unreachable(
             "Not all delete instances handled for Patch derived classes");
       case DebugPatchKind::PatchValue32:
         delete reinterpret_cast<DebugPatch32 *>(P);
         break;
       case DebugPatchKind::PatchValue64to32:
         delete reinterpret_cast<DebugPatch64to32 *>(P);
         break;
+      case DebugPatchKind::PatchValue32GenericSize:
+        delete reinterpret_cast<DebugPatch32GenericSize *>(P);
+        break;
       case DebugPatchKind::PatchValue64:
         delete reinterpret_cast<DebugPatch64 *>(P);
         break;
       case DebugPatchKind::PatchValueVariable:
         delete reinterpret_cast<DebugPatchVariableSize *>(P);
         break;
       case DebugPatchKind::ReferencePatchValue:
         delete reinterpret_cast<DebugPatchReference *>(P);

bolt/lib/Core/DebugData.cpp

 }
 
 void DebugInfoBinaryPatcher::addLE32Patch(uint64_t Offset, uint32_t NewValue,
                                           uint32_t OldValueSize) {
   Offset -= DWPUnitOffset;
   std::lock_guard<std::mutex> Lock(WriterMutex);
   if (OldValueSize == 4)
     DebugPatches.emplace_back(new DebugPatch32(Offset, NewValue));
-  else
+  else if (OldValueSize == 8)
     DebugPatches.emplace_back(new DebugPatch64to32(Offset, NewValue));
+  else
+    DebugPatches.emplace_back(
+        new DebugPatch32GenericSize(Offset, NewValue, OldValueSize));
 }
 
 void SimpleBinaryPatcher::addBinaryPatch(uint64_t Offset,
                                          std::string &&NewValue,
                                          uint32_t OldValueSize) {
   Patches.emplace_back(Offset, std::move(NewValue));
 }
 
for (UniquePatchPtrType &PatchBase : DebugPatches) {
     Patch *P = PatchBase.get();
     switch (P->Kind) {
     default:
       continue;
     case DebugPatchKind::PatchValue64to32: {
       PreviousChangeInSize -= 4;
       break;
     }
+    case DebugPatchKind::PatchValue32GenericSize: {
+      DebugPatchVariableSize *DPVS =
+          reinterpret_cast<DebugPatchVariableSize *>(P);
+      PreviousChangeInSize += 4 - DPVS->OldValueSize;
+      break;
+    }
     case DebugPatchKind::PatchValueVariable: {
       DebugPatchVariableSize *DPV =
           reinterpret_cast<DebugPatchVariableSize *>(P);
       std::string Temp;
       raw_string_ostream OS(Temp);
       encodeULEB128(DPV->Value, OS);
       PreviousChangeInSize += Temp.size() - DPV->OldValueSize;
       break;
for (const UniquePatchPtrType &PatchBase : DebugPatches) {
     }
     case DebugPatchKind::PatchValue64to32: {
       DebugPatch64to32 *P64to32 = reinterpret_cast<DebugPatch64to32 *>(P);
       Offset = P64to32->Offset;
       OldValueSize = 8;
       ByteSequence = encodeLE(4, P64to32->Value);
       break;
     }
+    case DebugPatchKind::PatchValue32GenericSize: {
+      DebugPatchVariableSize *DPVS =
+          reinterpret_cast<DebugPatchVariableSize *>(P);
+      Offset = DPVS->Offset;
+      OldValueSize = DPVS->OldValueSize;
+      ByteSequence = encodeLE(4, DPVS->Value);
+      break;
+    }
     case DebugPatchKind::PatchValueVariable: {
       DebugPatchVariableSize *PV =
           reinterpret_cast<DebugPatchVariableSize *>(P);
       Offset = PV->Offset;
       OldValueSize = PV->OldValueSize;
       ByteSequence.clear();
       raw_string_ostream OS(ByteSequence);
       encodeULEB128(PV->Value, OS);

bolt/test/X86/high_pc_udata.s

+# REQUIRES: system-linux
+
+# RUN: llvm-mc -filetype=obj -triple x86_64-unknown-linux %s -o %t1.o
+# RUN: %clang %cflags %t1.o -o %t.exe -Wl,-q
+# RUN: llvm-bolt %t.exe -o %t.bolt -update-debug-sections
+# RUN: llvm-dwarfdump --show-form --verbose --debug-info %t.exe | FileCheck --check-prefix=PRECHECK %s
+# RUN: llvm-dwarfdump --show-form --verbose --debug-info %t.bolt | FileCheck --check-prefix=POSTCHECK %s
+
+# PRECHECK: DW_AT_high_pc [DW_FORM_udata] (15)
+# PRECHECK-NEXT: DW_AT_name [DW_FORM_strp]
+# PRECHECK-SAME: "main.cpp"
+
+# POSTCHECK: DW_AT_ranges [DW_FORM_sec_offset]
+# POSTCHECK-NEXT: [
+# POSTCHECK-NEXT: DW_AT_name [DW_FORM_strp]
+# POSTCHECK-SAME: "main.cpp"
+
+# Testing that BOLT transforms DW_AT_high_pc of form DW_FORM_udata correctly into DW_AT_ranges.
+# Manually changed so that DW_AT_high_pc is DW_FORM_udata, and that DW_AT_name is after it.
+# int main() {
+#    return 0;
+# }
+.text
+  .file "main.cpp"
+  .globl  main                            # -- Begin function main
+  .p2align  4, 0x90
+  .type main,@function
+main:                                   # @main
+.Lfunc_begin0:
+  .file 1 "" "main.cpp"
+  .loc  1 1 0                           # main.cpp:1:0
+  .cfi_startproc
+# %bb.0:                                # %entry
+  pushq %rbp
+  .cfi_def_cfa_offset 16
+  .cfi_offset %rbp, -16
+  movq  %rsp, %rbp
+  .cfi_def_cfa_register %rbp
+  movl  $0, -4(%rbp)
+.Ltmp0:
+  .loc  1 2 1 prologue_end              # main.cpp:2:1
+  xorl  %eax, %eax
+  popq  %rbp
+  .cfi_def_cfa %rsp, 8
+  retq
+.Ltmp1:
+.Lfunc_end0:
+  .size main, .Lfunc_end0-main
+  .cfi_endproc
+                                        # -- End function
+  .section  .debug_abbrev,"",@progbits
+  .byte 1                               # Abbreviation Code
+  .byte 17                              # DW_TAG_compile_unit
+  .byte 1                               # DW_CHILDREN_yes
+  .byte 37                              # DW_AT_producer
+  .byte 14                              # DW_FORM_strp
+  .byte 19                              # DW_AT_language
+  .byte 5                               # DW_FORM_data2
+  .byte 16                              # DW_AT_stmt_list
+  .byte 23                              # DW_FORM_sec_offset
+  .byte 27                              # DW_AT_comp_dir
+  .byte 14                              # DW_FORM_strp
+  .byte 17                              # DW_AT_low_pc
+  .byte 1                               # DW_FORM_addr
+  .byte 18                              # DW_AT_high_pc
+  .byte 15                              # DW_FORM_udata
+  .byte 3                               # DW_AT_name
+  .byte 14                              # DW_FORM_strp
+  .byte 0                               # EOM(1)
+  .byte 0                               # EOM(2)
+  .byte 2                               # Abbreviation Code
+  .byte 46                              # DW_TAG_subprogram
+  .byte 0                               # DW_CHILDREN_no
+  .byte 17                              # DW_AT_low_pc
+  .byte 1                               # DW_FORM_addr
+  .byte 18                              # DW_AT_high_pc
+  .byte 6                               # DW_FORM_data4
+  .byte 64                              # DW_AT_frame_base
+  .byte 24                              # DW_FORM_exprloc
+  .byte 3                               # DW_AT_name
+  .byte 14                              # DW_FORM_strp
+  .byte 58                              # DW_AT_decl_file
+  .byte 11                              # DW_FORM_data1
+  .byte 59                              # DW_AT_decl_line
+  .byte 11                              # DW_FORM_data1
+  .byte 73                              # DW_AT_type
+  .byte 19                              # DW_FORM_ref4
+  .byte 63                              # DW_AT_external
+  .byte 25                              # DW_FORM_flag_present
+  .byte 0                               # EOM(1)
+  .byte 0                               # EOM(2)
+  .byte 3                               # Abbreviation Code
+  .byte 36                              # DW_TAG_base_type
+  .byte 0                               # DW_CHILDREN_no
+  .byte 3                               # DW_AT_name
+  .byte 14                              # DW_FORM_strp
+  .byte 62                              # DW_AT_encoding
+  .byte 11                              # DW_FORM_data1
+  .byte 11                              # DW_AT_byte_size
+  .byte 11                              # DW_FORM_data1
+  .byte 0                               # EOM(1)
+  .byte 0                               # EOM(2)
+  .byte 0                               # EOM(3)
+  .section  .debug_info,"",@progbits
+.Lcu_begin0:
+  .long .Ldebug_info_end0-.Ldebug_info_start0 # Length of Unit
+.Ldebug_info_start0:
+  .short  4                               # DWARF version number
+  .long .debug_abbrev                   # Offset Into Abbrev. Section
+  .byte 8                               # Address Size (in bytes)
+  .byte 1                               # Abbrev [1] 0xb:0x40 DW_TAG_compile_unit
+  .long .Linfo_string0                  # DW_AT_producer
+  .short  33                              # DW_AT_language
+  .long .Lline_table_start0             # DW_AT_stmt_list
+  .long .Linfo_string2                  # DW_AT_comp_dir
+  .quad .Lfunc_begin0                   # DW_AT_low_pc
+  .byte .Lfunc_end0-.Lfunc_begin0       # DW_AT_high_pc
+  .long .Linfo_string1                  # DW_AT_name
+  .byte 2                               # Abbrev [2] 0x2a:0x19 DW_TAG_subprogram
+  .quad .Lfunc_begin0                   # DW_AT_low_pc
+  .long .Lfunc_end0-.Lfunc_begin0       # DW_AT_high_pc
+  .byte 1                               # DW_AT_frame_base
+  .byte 86
+  .long .Linfo_string3                  # DW_AT_name
+  .byte 1                               # DW_AT_decl_file
+  .byte 1                               # DW_AT_decl_line
+  .long 67                              # DW_AT_type
+                                        # DW_AT_external
+  .byte 3                               # Abbrev [3] 0x43:0x7 DW_TAG_base_type
+  .long .Linfo_string4                  # DW_AT_name
+  .byte 5                               # DW_AT_encoding
+  .byte 4                               # DW_AT_byte_size
+  .byte 0                               # End Of Children Mark
+.Ldebug_info_end0:
+  .section  .debug_str,"MS",@progbits,1
+.Linfo_string0:
+  .asciz  "clang" # string offset=0
+.Linfo_string1:
+  .asciz  "main.cpp"                      # string offset=134
+.Linfo_string2:
+  .asciz  "test" # string offset=143
+.Linfo_string3:
+  .asciz  "main"                          # string offset=186
+.Linfo_string4:
+  .asciz  "int"                           # string offset=191
+  .ident  "clang"
+  .section  ".note.GNU-stack","",@progbits
+  .addrsig
+  .section  .debug_line,"",@progbits
+.Lline_table_start0: