[PATCH] D61584: [DebugInfo] Some fields do not need relocations even relax is enabled.

Hsiangkai Wang via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat Jun 22 09:02:31 PDT 2019


HsiangKai marked an inline comment as done.
HsiangKai added a comment.

In D61584#1550273 <https://reviews.llvm.org/D61584#1550273>, @lewis-revill wrote:

> Just a quick comment, I'm assuming this depends on D58335 <https://reviews.llvm.org/D58335> based on the test file you've modified?


Yes, the test case is based on D58335 <https://reviews.llvm.org/D58335>.



================
Comment at: lib/MC/MCExpr.cpp:610
   if (Asm &&
-      (InSet || !Asm->getBackend().requiresDiffExpressionRelocations())) {
+      (InSet || (LHS_A && !LHS_A->getSymbol().getSection().hasInstructions()) ||
+       (LHS_B && !LHS_B->getSymbol().getSection().hasInstructions()) ||
----------------
edward-jones wrote:
> I found a bug with these latest changes when compiling the following program:
> 
> ```
> func:
>   .cfi_startproc
>   .cfi_endproc
> ```
> 
> With the command:
> 
> `llvm-mc -filetype=obj tmp.s -o tmp.o -triple riscv32`
> 
> The calls to `...->getSymbol().getSection()` can trigger an assertion if the symbol is not defined in a section, which seems to happen in the above testcase.
Thank you for your information. I have fixed the bug. Please help me to review it and verify the solution. Thanks.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D61584





More information about the llvm-commits mailing list