[PATCH] D124121: [DebugInfo] Give warning instead of error for premature terminator in .debug_aranges section.

[Junfeng Dong via Phabricator via llvm-commits]

junfd added inline comments.


================
Comment at: llvm/test/tools/llvm-profgen/premature-warning.s:1
+# REQUIRES: x86-registered-target
+# RUN: clang %s -o %t1
----------------
jhenderson wrote:
> I feel like the test name needs to include "aranges" somewhere in it, e.g. `premature-aranges-end-warning.s`
Make sense. I will change the name.


================
Comment at: llvm/test/tools/llvm-profgen/premature-warning.s:2
+# REQUIRES: x86-registered-target
+# RUN: clang %s -o %t1
+# RUN: llvm-profgen --binary=%t1 --output=%t2 --show-disassembly-only -x86-asm-syntax=intel --show-source-locations 2>&1 | FileCheck %s
----------------
jhenderson wrote:
> `clang` can't be used for tests under llvm/test, because it may not have been built. You should use llvm-mc (there are plenty of examples of the latter).
Will do. thanks for pointing this out.


================
Comment at: llvm/test/tools/llvm-profgen/premature-warning.s:5
+
+# CHECK: warning: address range table at
+
----------------
jhenderson wrote:
> I'd check the full warning, as this pattern could match other warnings from the aranges section.
Sure. I will change it.


================
Comment at: llvm/test/tools/llvm-profgen/premature-warning.s:7-15
+# the assembly code is from clang t.c -mllvm -generate-arange-section -g2 -save-temps
+# #include <stdio.h>
+# int main(){
+#   printf("hello world.\n");
+#   return 0;
+# }
+# then manually changed .debug_aragnes section
----------------
jhenderson wrote:
> In addition to the above, I'd note the version of clang you used to generate this assembly.
> 
> Up to you, but I'd consider using YAML as input instead of asm. For the simple test case here, you might find it leads to more readable and maintainable input, once you've done the initial legwork of writing the debug_info and debug_abbrev sections.
I saw some tests using YAML and coverts to obj file. could it be convert to executable file further? llvm-profgen require the binary input to be a valid executable.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124121/new/

https://reviews.llvm.org/D124121