[PATCH] D72027: [XCOFF][AIX] Support basic relocation type on AIX

[Jason Liu via Phabricator via llvm-commits]

jasonliu marked 8 inline comments as done.
jasonliu added inline comments.


================
Comment at: llvm/lib/MC/XCOFFObjectWriter.cpp:414
+  if (Type == XCOFF::RelocationType::R_POS)
+    FixedValue = SectionMap[SymASec]->Address +
+                 (SymA.isDefined() ? Layout.getSymbolOffset(SymA) : 0);
----------------
hubert.reinterpretcast wrote:
> hubert.reinterpretcast wrote:
> > Consider:
> > ```
> > int x = 2, arr[100] = { 1 };
> > extern int *p = arr + 42;
> > ```
> > 
> > The raw data with this patch does not reflect the offset from `arr` that `p` points to (and neither does the relocation itself):
> > ```
> > 00000004 arr:
> >        4: 00 00 00 01                   <unknown>
> >                 ...
> > 
> > 00000194 p:
> >      194: 00 00 00 04                   <unknown>
> > ```
> > 
> > The raw data without this patch was correct.
> > 
> As discussed off-list, the added test should cover this too.
Added into the same test. See arr and p in the test below. Thanks. 


================
Comment at: llvm/lib/MC/XCOFFObjectWriter.cpp:409
+    // plus any constant value that we might get.
+    FixedValue = SectionMap[SymASec]->Address +
+                 (SymA.isDefined() ? Layout.getSymbolOffset(SymA) : 0) +
----------------
jasonliu wrote:
> jasonliu wrote:
> > hubert.reinterpretcast wrote:
> > > Is `SectionMap[SymASec]->Address` a fancy way of saying `0` for undefined symbols? If so, can we move it into the ternary expression?
> > SectionMap[SymASec]->Address is not just for undefined symbols. If SymA is a label inside of csect, we need to get (the csect address + the symbol offset) to get the exact address of that label. 
> Had offline discussion on this, and I will change the code to this as suggested.
> 
> ```
> FixedValue = (SymA.isDefined() ? SectionMap[SymASec]->Address + Layout.getSymbolOffset(SymA) 
>                                                        : 0) + Target.getConstant();
> ```
> 
Just discovered why I could not do the way as suggested. Added rationale into the comments. 


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

https://reviews.llvm.org/D72027