[PATCH] D83751: [MC] Support .reloc sym+constant, *, *

Stefan Pintilie via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jul 14 11:21:22 PDT 2020


stefanp added inline comments.


================
Comment at: llvm/lib/MC/MCObjectStreamer.cpp:704
+        MCFixup::create(SRE.getSymbol().getOffset() + OffsetVal.getConstant(),
+                        Expr, Kind, Loc));
+    return None;
----------------
MaskRay wrote:
> stefanp wrote:
> > One of the issues I noticed with this situation is that the symbol may not be in the same data fragment as the `.reloc` directive. The code you have assumes that it is in the same DF.
> > 
> > I used the same DF as the symbol:
> > ```
> >   MCFragment *Fragment = Symbol->getSymbol().getFragment();
> >   if (!Fragment || Fragment->getKind() != MCFragment::FT_Data)
> >     return false;
> > ```
> GNU as somehow allows the following (it behaves as if .reloc is placed in `.data`)
> 
> ```
> .text
> .reloc .data, R_X86_64_NONE, 0
> 
> .data
> .globl 
> data:
> ```
> 
> It needs thoughts how to handle/detect this. This probably can go to a separate change.
I wouldn't know where to start with that `.data` reloc because I don't think we would be able to get the DF for it. I almost feel like we would have to make changes to `ELFObjectWriter::recordRelocation` and at least allow `MCFixup::create` to accept more than `uint32_t` as the offset. But that is a lot of work. Definitely a different patch.

Either way, I don't need the `.data` but I will need to cover the situation where the DF is different for the symbol. This is for code that will be coming from me later on. For this patch if you want to leave it as a TODO I'm ok with that. I can try to address it in the rework of D79625.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83751





More information about the llvm-commits mailing list