[PATCH] D126257: Round up zero-sized symbols to 1 byte in `.debug_aranges`.

Thu May 26 11:47:59 PDT 2022

dblaikie added a comment.

In D126257#3539510 <https://reviews.llvm.org/D126257#3539510>, @DavidSpickett wrote:

> In https://reviews.llvm.org/rG38eb4fe74b38 I moved the generic test into the X86 folder as it uses an X86 triple. You could think of the folder names there having 2 meanings, 1: the tests look at things specific to that arch and 2: they use that arch's triple (I think X86 is usually the one people choose if they want to test something generic anyway).

Thanks for the catch/cleanup!

================
Comment at: llvm/test/CodeGen/Generic/dwarf-aranges-zero-size.ll:18
+!0 = !DIGlobalVariableExpression(var: !1, expr: !DIExpression())
+!1 = distinct !DIGlobalVariable(name: "EXAMPLE", linkageName: "EXAMPLE", scope: null, file: null, line: 161, type: !2, isLocal: false, isDefinition: true, align: 1)
+!2 = !DIBasicType(name: "()", encoding: DW_ATE_unsigned)
----------------
bjope wrote:
> Notice that align here is specified in bits. I think it is a bit weird to have a 1 bit alignment?
> 
> So why do I care? Our downstream fork is checking that alignment is a multiple of byte size since the getAlignInBytes methods in DebugInfoMetadata.h is dividing the alignment specified in bits by the byte size. This test case hit such assertions.
> 
> Do you think it is ok to change this to "align: 8"?
Guessing we could probably just remove the "align: " field entirely here.

Usually I'd advocate for generating the example from some C++ source code fed into clang, since that's the most canonical IR we have - a small example with a zero-length array or the like. Though that might lead to more type information than is really helpful in a test like this.

I guess this was generated by Rust? So might be worth seeing why/where this alignment was created, maybe there's some bugs there to be sorted out too.

As for the alignment restrictions: Might be worth upstreaming your check into LLVM's debug info verifier to make it an explicit constraint of the IR - would make it easier to detect these sort of things sooner.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126257