[llvm-branch-commits] [lld] [ELF] Orphan placement: remove hasInputSections condition (PR #93761)

[Peter Smith via llvm-branch-commits]

smithp35 wrote:

I think https://github.com/llvm/llvm-project/pull/94519 looks good. Probably 
worth landing that first and rebasing this one.

https://github.com/llvm/llvm-project/pull/93761
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm-objdump] -r: support CREL (PR #97382)

[Peter Smith via llvm-branch-commits]

@@ -2687,6 +2687,15 @@ void Dumper::printRelocations() {
<< "VALUE\n";
 
 for (SectionRef Section : P.second) {
+  // CREL requires decoding and has its specific errors.
+  if (O.isELF() && ELFSectionRef(Section).getType() == ELF::SHT_CREL) {
+const ELFObjectFileBase *ELF = cast(&O);
+StringRef Err = ELF->getCrelError(Section);
+if (!Err.empty()) {
+  reportUniqueWarning(Err);

smithp35 wrote:

If we are giving a warning rather than an error, then perhaps we should use 
something like `getCrelDecodeProblem`

Just looks a bit strange recording errors, but then only reporting a warning.

My expectation for a decode error would be that we report it, but continue as 
best we can. I would expect a non-zero error code though. Please ignore if this 
isn't llvm-objdump practice. 

https://github.com/llvm/llvm-project/pull/97382
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm-objdump] -r: support CREL (PR #97382)

[Peter Smith via llvm-branch-commits]

@@ -292,6 +295,11 @@ template  class ELFObjectFile : public 
ELFObjectFileBase {
   const Elf_Shdr *DotSymtabSec = nullptr; // Symbol table section.
   const Elf_Shdr *DotSymtabShndxSec = nullptr; // SHT_SYMTAB_SHNDX section.
 
+  // Hold CREL relocations for SectionRef::relocations().
+  mutable SmallVector, 0> Crels;
+  // Hold CREL decoding errors.
+  mutable SmallVector CrelErrs;

smithp35 wrote:

I expect that most objects would have no decoding errors, yet it looks like 
we're allocating a potentially large (intermediate ELF file as output of LTO 
for example) amount of empty strings.

An alternative could be an unordered_map of section indexes to strings. This 
would also look a bit cleaner than just checking if the string is empty for no 
errors.

I don't have a strong opinion though.

https://github.com/llvm/llvm-project/pull/97382
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm-objdump] -r: support CREL (PR #97382)

[Peter Smith via llvm-branch-commits]

@@ -1453,6 +1525,15 @@ template  bool 
ELFObjectFile::isRelocatableObject() const {
   return EF.getHeader().e_type == ELF::ET_REL;
 }
 
+template 
+StringRef ELFObjectFile::getCrelError(DataRefImpl Sec) const {
+  uintptr_t SHT = reinterpret_cast(cantFail(EF.sections()).begin());
+  auto I = (Sec.p - SHT) / EF.getHeader().e_shentsize;
+  if (I < CrelErrs.size())
+return CrelErrs[I];
+  return "";

smithp35 wrote:

Wouldn't this be an error (or perhaps internal error) for an out of bounds 
access?

https://github.com/llvm/llvm-project/pull/97382
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm-objdump] -r: support CREL (PR #97382)

[Peter Smith via llvm-branch-commits]

https://github.com/smithp35 edited 
https://github.com/llvm/llvm-project/pull/97382
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm-objdump] -r: support CREL (PR #97382)

[Peter Smith via llvm-branch-commits]

@@ -2687,6 +2687,15 @@ void Dumper::printRelocations() {
<< "VALUE\n";
 
 for (SectionRef Section : P.second) {
+  // CREL requires decoding and has its specific errors.

smithp35 wrote:

I recommend
```
// CREL sections require decoding, each section may have its own specific 
errors.
```

https://github.com/llvm/llvm-project/pull/97382
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm-objdump] -r: support CREL (PR #97382)

[Peter Smith via llvm-branch-commits]

https://github.com/smithp35 commented:

I agree with jh7370 that it would be good to extract the decoding logic, it 
seems like each tool needs some way of passing in a way to step through the 
data, return the decoded data, and a way of storing errors. It is possible that 
this will end up being more code than just inlining the algorithm, however I'm 
hoping that this additional code is simpler and less error prone than copying 
the logic.

May be worth a try to see what it looks like.

https://github.com/llvm/llvm-project/pull/97382
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm-objdump] -r: support CREL (PR #97382)

[Peter Smith via llvm-branch-commits]

@@ -207,6 +209,43 @@ bool isSectionInSegment(const typename ELFT::Phdr &Phdr,
  checkSectionVMA(Phdr, Sec);
 }
 
+template 

smithp35 wrote:

Thanks for lifting this out. Possibly worth a comment describing HdrHandler and 
EntryHandler. For example:
```
// The HdrHandler is called once with the number of relocations and whether the 
relocations have addends.
// The EntryHandler is called once per decoded relocation.

https://github.com/llvm/llvm-project/pull/97382
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm-objdump] -r: support CREL (PR #97382)

[Peter Smith via llvm-branch-commits]

https://github.com/smithp35 commented:

Thanks for the updates. Only a couple of small suggestions. 

Will be out of office till Monday next week. I'm fine with others approving.

https://github.com/llvm/llvm-project/pull/97382
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm-objdump] -r: support CREL (PR #97382)

[Peter Smith via llvm-branch-commits]

@@ -207,6 +209,43 @@ bool isSectionInSegment(const typename ELFT::Phdr &Phdr,
  checkSectionVMA(Phdr, Sec);
 }
 
+template 
+Error decodeCrel(ArrayRef Content,
+ function_ref HdrHandler,

smithp35 wrote:

could be worth 
```
uint64_t /* relocation count */, bool /* explicit addends */
```

https://github.com/llvm/llvm-project/pull/97382
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm-objdump] -r: support CREL (PR #97382)

[Peter Smith via llvm-branch-commits]

https://github.com/smithp35 edited 
https://github.com/llvm/llvm-project/pull/97382
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm-objcopy] Support CREL (PR #97521)

[Peter Smith via llvm-branch-commits]

https://github.com/smithp35 commented:

Only a couple of small comments from me. I'll be out of the office till Monday 
next week, I'm fine for others to progress this wihout me.

https://github.com/llvm/llvm-project/pull/97521
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm-objcopy] Support CREL (PR #97521)

[Peter Smith via llvm-branch-commits]

@@ -0,0 +1,60 @@
+//===- MCELFExtras.h - Extra functions for ELF --*- C++ 
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_MC_MCELFEXTRAS_H
+#define LLVM_MC_MCELFEXTRAS_H
+
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/BinaryFormat/ELF.h"
+#include "llvm/Support/LEB128.h"
+#include "llvm/Support/raw_ostream.h"
+
+#include 
+#include 
+
+namespace llvm::ELF {

smithp35 wrote:

I think it would be helpful to document the interface of ToCrel
// ToCrel is responsible for converting a const &RelocsTy to a Elf_Crel

https://github.com/llvm/llvm-project/pull/97521
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm-objcopy] Support CREL (PR #97521)

[Peter Smith via llvm-branch-commits]

https://github.com/smithp35 edited 
https://github.com/llvm/llvm-project/pull/97521
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm-objcopy] Support CREL (PR #97521)

[Peter Smith via llvm-branch-commits]

@@ -1861,7 +1886,15 @@ template  Error 
ELFBuilder::readSections(bool EnsureSymtab) {
 
   const typename ELFFile::Elf_Shdr *Shdr =
   Sections->begin() + RelSec->Index;
-  if (RelSec->Type == SHT_REL) {
+  if (RelSec->Type == SHT_CREL) {
+auto Rels = ElfFile.crels(*Shdr);

smithp35 wrote:

Would `RelsOrRelas` be a better name as it will make the meaning of first and 
second more obvious at the point of use?

https://github.com/llvm/llvm-project/pull/97521
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] [llvm-objcopy] Support CREL (PR #97521)

[Peter Smith via llvm-branch-commits]

https://github.com/smithp35 approved this pull request.

Thanks for the updates LGTM too. I'm fine with committing the extracted code 
separately.

https://github.com/llvm/llvm-project/pull/97521
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm-objdump] -r: support CREL (PR #97382)

[Peter Smith via llvm-branch-commits]

https://github.com/smithp35 approved this pull request.

LGTM too, thanks for the updates.

https://github.com/llvm/llvm-project/pull/97382
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [lld] [llvm] release/19.x: [ARM] Create mapping symbols with non-unique names (PR #100171)

[Peter Smith via llvm-branch-commits]

https://github.com/smithp35 approved this pull request.

LGTM, this is a simple and safe change.

https://github.com/llvm/llvm-project/pull/100171
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [lld] release/19.x: [ELF] Support relocatable files using CREL with explicit addends (PR #101532)

[Peter Smith via llvm-branch-commits]

https://github.com/smithp35 approved this pull request.

CREL is an experimental optional feature that we want to get feedback on, it 
would be good to get this into the 19 release.

https://github.com/llvm/llvm-project/pull/101532
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [ELF] Place .lbss/.lrodata/.ldata after .bss (PR #81224)

[Peter Smith via llvm-branch-commits]

@@ -1124,11 +1127,15 @@ template  void 
Writer::setReservedSymbolSections() {
   }
 
   if (last) {
-// _edata points to the end of the last mapped initialized section.
+// _edata points to the end of the last mapped initialized section before

smithp35 wrote:

Apart from the comment about .ldata may be after _edata, is this something we 
had wrong prior to this patch?

If so it could be worth splitting out into a separate patch so we can 
identify/test it separately. Ideally we'd want a test case for each of the 
conditions below. 

https://github.com/llvm/llvm-project/pull/81224
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [ELF] Place .lbss/.lrodata/.ldata after .bss (PR #81224)

[Peter Smith via llvm-branch-commits]

https://github.com/smithp35 edited 
https://github.com/llvm/llvm-project/pull/81224
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [ELF] Place .lbss/.lrodata/.ldata after .bss (PR #81224)

[Peter Smith via llvm-branch-commits]

https://github.com/smithp35 commented:

Matching GNU ld seems sensible to me, although best get this reviewed by 
someone with an X86_64 background.

https://github.com/llvm/llvm-project/pull/81224
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [ARM, MC] Support FDPIC relocations (PR #82187)

[Peter Smith via llvm-branch-commits]

https://github.com/smithp35 edited 
https://github.com/llvm/llvm-project/pull/82187
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [ARM, MC] Support FDPIC relocations (PR #82187)

[Peter Smith via llvm-branch-commits]

@@ -223,6 +223,12 @@ class MCSymbolRefExpr : public MCExpr {
 VK_SECREL,
 VK_SIZE,// symbol@SIZE
 VK_WEAKREF, // The link between the symbols in .weakref foo, bar
+VK_FUNCDESC,

smithp35 wrote:

While VK_TLSGD_FDPIC are somewhat self documenting, it may be worth a comment 
for VK_FUNCDESC to VK_GOTOFFUNCDESC to state that these are for FDPIC.

https://github.com/llvm/llvm-project/pull/82187
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [ARM, MC] Support FDPIC relocations (PR #82187)

[Peter Smith via llvm-branch-commits]

https://github.com/smithp35 commented:

I've checked over the implementation with binutils. Out of interest are you 
planning on implementing all of fdpic or just enough to get assembler/linker 
support working?

If you are there are some other GNU options that may be useful to look at as 
possible intermediate steps. In particular -mno-pic-data-is-text-relative 
https://gcc.gnu.org/onlinedocs/gcc/ARM-Options.html#index-mpic-data-is-text-relative
 this uses a single static base for the GOT which is sufficient for position 
independent executables but no shared library support.

https://github.com/llvm/llvm-project/pull/82187
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [ARM, MC] Support FDPIC relocations (PR #82187)

[Peter Smith via llvm-branch-commits]

@@ -11358,6 +11361,37 @@ bool ARMAsmParser::parseDirectiveARM(SMLoc L) {
   return false;
 }
 
+MCSymbolRefExpr::VariantKind
+ARMAsmParser::getVariantKindForName(StringRef Name) const {
+  return StringSwitch(Name.lower())
+  .Case("funcdesc", MCSymbolRefExpr::VK_FUNCDESC)
+  .Case("got", MCSymbolRefExpr::VK_GOT)
+  .Case("got_prel", MCSymbolRefExpr::VK_ARM_GOT_PREL)
+  .Case("gotfuncdesc", MCSymbolRefExpr::VK_GOTFUNCDESC)
+  .Case("gotoff", MCSymbolRefExpr::VK_GOTOFF)
+  .Case("gotofffuncdesc", MCSymbolRefExpr::VK_GOTOFFFUNCDESC)
+  .Case("gottpoff", MCSymbolRefExpr::VK_GOTTPOFF)
+  .Case("gottpoff_fdpic", MCSymbolRefExpr::VK_GOTTPOFF_FDPIC)
+  .Case("imgrel", MCSymbolRefExpr::VK_COFF_IMGREL32)
+  .Case("none", MCSymbolRefExpr::VK_ARM_NONE)
+  .Case("plt", MCSymbolRefExpr::VK_PLT)
+  .Case("prel31", MCSymbolRefExpr::VK_ARM_PREL31)
+  .Case("sbrel", MCSymbolRefExpr::VK_ARM_SBREL)
+  .Case("secrel32", MCSymbolRefExpr::VK_SECREL)
+  .Case("target1", MCSymbolRefExpr::VK_ARM_TARGET1)
+  .Case("target2", MCSymbolRefExpr::VK_ARM_TARGET2)
+  .Case("tlscall", MCSymbolRefExpr::VK_TLSCALL)
+  .Case("tlsdesc", MCSymbolRefExpr::VK_TLSDESC)
+  .Case("tlsgd", MCSymbolRefExpr::VK_TLSGD)
+  .Case("tlsgd_fdpic", MCSymbolRefExpr::VK_TLSGD_FDPIC)
+  .Case("tlsld", MCSymbolRefExpr::VK_TLSLD)
+  .Case("tlsldm", MCSymbolRefExpr::VK_TLSLDM)
+  .Case("tlsldm_fdpic", MCSymbolRefExpr::VK_TLSLDM_FDPIC)
+  .Case("tlsldo", MCSymbolRefExpr::VK_ARM_TLSLDO)
+  .Case("tpoff", MCSymbolRefExpr::VK_TPOFF)
+  .Default(MCSymbolRefExpr::VK_Invalid);

smithp35 wrote:

I think Arm supports "tlddescseq" from checking this list against binutils. I 
don't think this was in the original list though.

https://github.com/llvm/llvm-project/pull/82187
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [ARM, MC] Support FDPIC relocations (PR #82187)

[Peter Smith via llvm-branch-commits]

@@ -11358,6 +11361,37 @@ bool ARMAsmParser::parseDirectiveARM(SMLoc L) {
   return false;
 }
 
+MCSymbolRefExpr::VariantKind
+ARMAsmParser::getVariantKindForName(StringRef Name) const {

smithp35 wrote:

I can see a design trade-off here. By implementing only the expressions that 
are supported on Arm we find some problems earlier, however we may miss out on 
generic expressions added to the `MCSymbolRefExpr::getVariantKindForName` but 
not for Arm. I can see these getting out of sync.

Would it be worth calling the MCSymbolRefExpr::getVariantKindForName if the 
Name isn't found? Ideally we could refactor so that 
MCSymbolRefExpr::getVariantKindForName contains only generic and no target 
specific variant kinds.



https://github.com/llvm/llvm-project/pull/82187
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [ARM, MC] Support FDPIC relocations (PR #82187)

[Peter Smith via llvm-branch-commits]

@@ -84,6 +84,11 @@ unsigned ARMELFObjectWriter::GetRelocTypeInner(const MCValue 
&Target,
   if (Kind >= FirstLiteralRelocationKind)
 return Kind - FirstLiteralRelocationKind;
   MCSymbolRefExpr::VariantKind Modifier = Target.getAccessVariant();
+  auto CheckFDPIC = [&]() {
+if (getOSABI() != ELF::ELFOSABI_ARM_FDPIC)
+  Ctx.reportError(Fixup.getLoc(),

smithp35 wrote:

I think the error message could be improved by giving out the string 
representation of the relocation code. For example:
```relocation R_ARM_FUNCDESC only supported in FDPIC mode```. I think this 
should be relatively easy to add.

https://github.com/llvm/llvm-project/pull/82187
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [ARM, MC] Support FDPIC relocations (PR #82187)

[Peter Smith via llvm-branch-commits]

https://github.com/smithp35 commented:

Thanks for the update, I'm happy with the changes. I would like to see more 
options for position independent code in embedded systems.

The other related embedded position independent option for microcontrollers 
that I'm aware of is `-fropi` and `-frwpi` which is already implemented in 
clang. This is relocation free with just one static data area accessed as an 
offset from r9. It works quite well for C code, but doesn't support C++ 
(vtables and RTTI need relocations) and some forms of static initialisation. 


https://github.com/llvm/llvm-project/pull/82187
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [ARM, MC] Support FDPIC relocations (PR #82187)

[Peter Smith via llvm-branch-commits]

https://github.com/smithp35 approved this pull request.

Forgot to approve

https://github.com/llvm/llvm-project/pull/82187
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [lld] PR for llvm/llvm-project#79339 (PR #79357)

[Peter Smith via llvm-branch-commits]

smithp35 wrote:

This is good to go for merging, a simple safe change.

https://github.com/llvm/llvm-project/pull/79357
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [lld] PR for llvm/llvm-project#79339 (PR #79357)

[Peter Smith via llvm-branch-commits]

https://github.com/smithp35 approved this pull request.

LGTM for merging

https://github.com/llvm/llvm-project/pull/79357
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [lld] ReleaseNotes: add lld/ELF notes (PR #80393)

[Peter Smith via llvm-branch-commits]

https://github.com/smithp35 commented:

What's there LGTM too, I've suggested a couple of possible additions that you 
might want to consider.


https://github.com/llvm/llvm-project/pull/80393
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [lld] ReleaseNotes: add lld/ELF notes (PR #80393)

[Peter Smith via llvm-branch-commits]

@@ -29,8 +29,42 @@ ELF Improvements
 * ``--fat-lto-objects`` option is added to support LLVM FatLTO.
   Without ``--fat-lto-objects``, LLD will link LLVM FatLTO objects using the
   relocatable object file. (`D146778 `_)
+* ``-Bsymbolic-non-weak`` is added to directly bind non-weak definitions.
+  (`D158322 `_)
+* ``--lto-validate-all-vtables-have-type-infos``, which complements
+  ``--lto-whole-program-visibility``, is added to disable unsafe whole-program
+  devirtualization. ``--lto-known-safe-vtables=`` can be used
+  to mark known-safe vtable symbols.
+  (`D155659 `_)

smithp35 wrote:

Could be worth adding
* ``--save-temps --lto-emit-asm`` now derives ELF/asm file names from bitcode 
file names. ld.lld --save-temps a.o d/b.o -o out` will create ELF relocatable 
files `out.lto.a.o`/`d/out.lto.b.o` instead of `out1.lto.o`/`out2.lto.o`. 
(`#78835 `_)


https://github.com/llvm/llvm-project/pull/80393
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [lld] ReleaseNotes: add lld/ELF notes (PR #80393)

[Peter Smith via llvm-branch-commits]

https://github.com/smithp35 edited 
https://github.com/llvm/llvm-project/pull/80393
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [lld] ReleaseNotes: add lld/ELF notes (PR #80393)

[Peter Smith via llvm-branch-commits]

@@ -29,8 +29,42 @@ ELF Improvements
 * ``--fat-lto-objects`` option is added to support LLVM FatLTO.
   Without ``--fat-lto-objects``, LLD will link LLVM FatLTO objects using the
   relocatable object file. (`D146778 `_)
+* ``-Bsymbolic-non-weak`` is added to directly bind non-weak definitions.
+  (`D158322 `_)
+* ``--lto-validate-all-vtables-have-type-infos``, which complements
+  ``--lto-whole-program-visibility``, is added to disable unsafe whole-program
+  devirtualization. ``--lto-known-safe-vtables=`` can be used
+  to mark known-safe vtable symbols.
+  (`D155659 `_)
+* ``--no-allow-shlib-undefined`` now reports errors for DSO referencing
+  non-exported definitions.
+  (`#70769 `_)
 * common-page-size can now be larger than the system page-size.
   (`#57618 `_)
+* When call graph profile information is availablue due to instrumentation or
+  sample PGO, input sections are now sorted using the new ``cdsort`` algorithm,
+  better than the previous ``hfsort`` algorithm.
+  (`D152840 `_)
+* Symbol assignments like ``a = DEFINED(a) ? a : 0;`` are now handled.
+  (`#65866 `_)
+* ``OVERLAY`` now supports optional start address and LMA
+  (`#77272 `_)
+* Relocations referencing a symbol defined in ``/DISCARD/`` section now lead to
+  an error.
+  (`#69295 `_)
+* For AArch64 MTE, global variable descriptors have been implemented.
+  (`D152921 `_)
+* ``R_LARCH_PCREL20_S2``/``R_LARCH_ADD6``/``R_LARCH_CALL36`` and extreme code
+  model relocations are now supported.
+* ``--emit-relocs`` is now supported for RISC-V linker relaxation.
+  (`D159082 `_)
+* Call relaxation respects RVC when mixing +c and -c relocatable files.
+  (`#73977 `_)
+* ``R_RISCV_SET_ULEB128``/``R_RISCV_SUB_ULEB128`` relocations are now 
supported.
+  (`#72610 `_)
+  (`#77261 `_)
+* RISC-V TLSDESC is now supported.
+  (`#79239 `_)
 

smithp35 wrote:

Could be worth adding
* R_AARCH64_GOTPCREL32 is now supported. (`#72584 
`_)

https://github.com/llvm/llvm-project/pull/80393
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [lld] ReleaseNotes: add lld/ELF notes (PR #80393)

[Peter Smith via llvm-branch-commits]

https://github.com/smithp35 edited 
https://github.com/llvm/llvm-project/pull/80393
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [lld] ReleaseNotes: add lld/ELF notes (PR #80393)

[Peter Smith via llvm-branch-commits]

https://github.com/smithp35 edited 
https://github.com/llvm/llvm-project/pull/80393
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [Driver] Add -Wa, options -mmapsyms={default, implicit} (PR #104542)

[Peter Smith via llvm-branch-commits]

@@ -7131,6 +7131,8 @@ def massembler_fatal_warnings : Flag<["-"], 
"massembler-fatal-warnings">,
 def crel : Flag<["--"], "crel">,
   HelpText<"Enable CREL relocation format (ELF only)">,
   MarshallingInfoFlag>;
+def mmapsyms_implicit : Flag<["-"], "mmapsyms=implicit">,

smithp35 wrote:

I think it would be useful to have similar help text to llvm-mc 
(https://github.com/llvm/llvm-project/pull/99718/files#diff-e84c9aa6b25b1a4fe2047de3a32ab330e945d2944b14451d310e4b706a39cbafR140)
 

https://github.com/llvm/llvm-project/pull/104542
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [Driver] Add -Wa, options -mmapsyms={default, implicit} (PR #104542)

[Peter Smith via llvm-branch-commits]

https://github.com/smithp35 edited 
https://github.com/llvm/llvm-project/pull/104542
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [Driver] Add -Wa, options -mmapsyms={default, implicit} (PR #104542)

[Peter Smith via llvm-branch-commits]

https://github.com/smithp35 commented:

I think we could do with some help text. Otherwise code changes look good.

https://github.com/llvm/llvm-project/pull/104542
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [Driver] Add -Wa, options -mmapsyms={default, implicit} (PR #104542)

[Peter Smith via llvm-branch-commits]

https://github.com/smithp35 approved this pull request.

Thanks for adding the help. LGTM.

https://github.com/llvm/llvm-project/pull/104542
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [lld] [PAC][lld][AArch64][ELF] Support signed TLSDESC (PR #113817)

[Peter Smith via llvm-branch-commits]

smithp35 wrote:

Just got back from vacation today. I plan to create a PR for the ABI tomorrow. 
Will take a look at this patch tomorrow.

https://github.com/llvm/llvm-project/pull/113817
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [lld] [PAC][lld] Use braa instr in PAC PLT sequence with valid PAuth core info (PR #113945)

[Peter Smith via llvm-branch-commits]

@@ -1096,8 +1113,10 @@ void AArch64BtiPac::writePlt(uint8_t *buf, const Symbol 
&sym,
   relocateNoSym(buf + 4, R_AARCH64_LDST64_ABS_LO12_NC, gotPltEntryAddr);
   relocateNoSym(buf + 8, R_AARCH64_ADD_ABS_LO12_NC, gotPltEntryAddr);
 
-  if (pacEntry)
-memcpy(buf + sizeof(addrInst), pacBr, sizeof(pacBr));
+  if (pacEntryKind != PEK_NoAuth)
+memcpy(buf + sizeof(addrInst),
+   pacEntryKind == PEK_AuthHint ? pacHintBr : pacBr,

smithp35 wrote:

Could be a possibility to make pacHintBr and pacBr into an array indexed by 
pacEntryKind.
```
memcpy(buf + sizeof(addrInst),
  pacBrInstrs[pacEntryKind],
  sizeof(pacBrInstrs[pacEntryKind]));
```

Not sure if it is worth the trouble though.

https://github.com/llvm/llvm-project/pull/113945
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [lld] [PAC][lld] Use braa instr in PAC PLT sequence with valid PAuth core info (PR #113945)

[Peter Smith via llvm-branch-commits]

@@ -1014,9 +1018,18 @@ AArch64BtiPac::AArch64BtiPac(Ctx &ctx) : AArch64(ctx) {
   // relocations.
   // The PAC PLT entries require dynamic loader support and this isn't known
   // from properties in the objects, so we use the command line flag.
-  pacEntry = ctx.arg.zPacPlt;

smithp35 wrote:

It could be worth adding to the comment here. Something like:
```
By default we only use hint-space instructions, but if we detect the PAuthABI, 
which requires v8.3-A we can use the non-hint space instructions.
```

https://github.com/llvm/llvm-project/pull/113945
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [lld] [PAC][lld] Use braa instr in PAC PLT sequence with valid PAuth core info (PR #113945)

[Peter Smith via llvm-branch-commits]

https://github.com/smithp35 edited 
https://github.com/llvm/llvm-project/pull/113945
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [lld] [PAC][lld] Use braa instr in PAC PLT sequence with valid PAuth core info (PR #113945)

[Peter Smith via llvm-branch-commits]

https://github.com/smithp35 commented:

No objections from me. A small suggestion for the comment and a possible 
simplification.

https://github.com/llvm/llvm-project/pull/113945
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [lld] [PAC][lld][AArch64][ELF] Support signed TLSDESC (PR #113817)

[Peter Smith via llvm-branch-commits]

https://github.com/smithp35 commented:

A few small suggestions from me.

https://github.com/llvm/llvm-project/pull/113817
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [lld] [PAC][lld][AArch64][ELF] Support signed TLSDESC (PR #113817)

[Peter Smith via llvm-branch-commits]

https://github.com/smithp35 edited 
https://github.com/llvm/llvm-project/pull/113817
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [lld] [PAC][lld][AArch64][ELF] Support signed TLSDESC (PR #113817)

[Peter Smith via llvm-branch-commits]

@@ -1352,6 +1352,36 @@ unsigned RelocationScanner::handleTlsRelocation(RelExpr 
expr, RelType type,
 return 1;
   }
 
+  auto fatalBothAuthAndNonAuth = [&sym]() {
+fatal("both AUTH and non-AUTH TLSDESC entries for '" + sym.getName() +

smithp35 wrote:

Can you add getLocation to the error message so that a user can find the source 
of at least one of the relocations?

I also recommend not using fatal but use error so that a user can get 
diagnostics like the map file out of the link.

https://github.com/llvm/llvm-project/pull/113817
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [lld] [PAC][lld][AArch64][ELF] Support signed TLSDESC (PR #113817)

[Peter Smith via llvm-branch-commits]

@@ -0,0 +1,134 @@
+// REQUIRES: aarch64
+// RUN: rm -rf %t && split-file %s %t && cd %t
+
+//--- a.s
+
+.section .tbss,"awT",@nobits
+.global a
+a:
+.xword 0
+
+//--- ok.s
+
+// RUN: llvm-mc -filetype=obj -triple=aarch64-pc-linux -mattr=+pauth ok.s -o 
ok.o
+// RUN: ld.lld -shared ok.o -o ok.so
+// RUN: llvm-objdump --no-print-imm-hex -d --no-show-raw-insn ok.so | \
+// RUN:   FileCheck -DP=20 -DA=896 -DB=912 -DC=928 %s
+// RUN: llvm-readobj -r -x .got ok.so | FileCheck --check-prefix=REL \
+// RUN:   -DP1=20 -DA1=380 -DB1=390 -DC1=3A0 -DP2=020 -DA2=380 -DB2=390 
-DC2=3a0 %s
+
+// RUN: llvm-mc -filetype=obj -triple=aarch64-pc-linux -mattr=+pauth a.s -o 
a.so.o
+// RUN: ld.lld -shared a.so.o -soname=so -o a.so
+// RUN: ld.lld ok.o a.so -o ok
+// RUN: llvm-objdump --no-print-imm-hex -d --no-show-raw-insn ok | \
+// RUN:   FileCheck -DP=220 -DA=936 -DB=952 -DC=968 %s
+// RUN: llvm-readobj -r -x .got ok | FileCheck --check-prefix=REL \
+// RUN:   -DP1=220 -DA1=3A8 -DB1=3B8 -DC1=3C8 -DP2=220 -DA2=3a8 -DB2=3b8 
-DC2=3c8 %s
+
+.text
+adrpx0, :tlsdesc_auth:a
+ldr x16, [x0, :tlsdesc_auth_lo12:a]
+add x0, x0, :tlsdesc_auth_lo12:a
+.tlsdesccall a
+blraa   x16, x0
+
+// CHECK:  adrpx0, 0x[[P]]000
+// CHECK-NEXT: ldr x16, [x0, #[[A]]]
+// CHECK-NEXT: add x0, x0, #[[A]]
+// CHECK-NEXT: blraa   x16, x0
+
+// Create relocation against local TLS symbols where linker should

smithp35 wrote:

I think LLD has a convention to put an additional comment character for lines 
that are not RUN or FileCheck comments. For example:
/// Create relocation ... 

https://github.com/llvm/llvm-project/pull/113817
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [lld] [PAC][lld][AArch64][ELF] Support signed TLSDESC (PR #113817)

[Peter Smith via llvm-branch-commits]

@@ -92,6 +92,10 @@ enum RelExpr {
   R_AARCH64_PAGE_PC,
   R_AARCH64_RELAX_TLS_GD_TO_IE_PAGE_PC,
   R_AARCH64_TLSDESC_PAGE,
+  R_AARCH64_AUTH_TLSDESC_PAGE,
+  // TODO: maybe it's better to rename this expression
+  // to avoid name conflict with dynamic reloc
+  R_AARCH64_AUTH_TLSDESC,

smithp35 wrote:

Could be `R_AARCH64_AUTH_TLSDESC_STATIC`? 

https://github.com/llvm/llvm-project/pull/113817
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [lld] release/20.x: [ELF] --package-metadata: support %[0-9a-fA-F][0-9a-fA-F] (PR #126549)

[Peter Smith via llvm-branch-commits]

https://github.com/smithp35 approved this pull request.

LGTM. This would be useful as it would be needed if Linux Distros start using 
this.

https://github.com/llvm/llvm-project/pull/126549
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [lld] ELF: Only rewrite non-preemptible IFUNCs to IPLT functions if a non-IRELATIVE relocation is needed. (PR #133531)

[Peter Smith via llvm-branch-commits]

@@ -1761,6 +1761,9 @@ void RelocationBaseSection::computeRels() {
 llvm::sort(nonRelative, irelative, [&](auto &a, auto &b) {
   return std::tie(a.r_sym, a.r_offset) < std::tie(b.r_sym, b.r_offset);
 });
+llvm::sort(irelative, relocs.end(), [&](auto &a, auto &b) {

smithp35 wrote:

Could be worth updating the comment on line 1753, which doesn't mention 
irelative after non-irelative. If the r_offset is not just for readability it 
will be worth updating that too. 

https://github.com/llvm/llvm-project/pull/133531
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [lld] ELF: Only rewrite non-preemptible IFUNCs to IPLT functions if a non-IRELATIVE relocation is needed. (PR #133531)

[Peter Smith via llvm-branch-commits]

@@ -1964,6 +1979,26 @@ void elf::postScanRelocations(Ctx &ctx) {
   for (ELFFileBase *file : ctx.objectFiles)
 for (Symbol *sym : file->getLocalSymbols())
   fn(*sym);
+
+  // Now that we have checked all ifunc symbols for demotion to regular 
function
+  // symbols, move IRELATIVE relocations to the right place:
+  // - Relocations for non-demoted ifuncs are added to .rela.dyn
+  // - Relocations for demoted ifuncs are turned into RELATIVE relocations
+  //   or static relocations in PDEs

smithp35 wrote:

Could you expand the acronym? I think this means Position Dependent Executable 
(PDE). It isn't used anywhere else in the codebase, and while derivable made me 
stop and think of alternatives.

https://github.com/llvm/llvm-project/pull/133531
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [lld] ELF: Only rewrite non-preemptible IFUNCs to IPLT functions if a non-IRELATIVE relocation is needed. (PR #133531)

[Peter Smith via llvm-branch-commits]

@@ -42,6 +42,8 @@ void printTraceSymbol(const Symbol &sym, StringRef name);
 enum {
   NEEDS_GOT = 1 << 0,
   NEEDS_PLT = 1 << 1,
+  // True if this is an ifunc with a direct relocation that cannot be

smithp35 wrote:

Although not new, could be worth expanding on what a direct relocation is in 
the comment. Could be just `direct (non GOT or PLT generating) relocation ...`  

https://github.com/llvm/llvm-project/pull/133531
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [lld] ELF: Only rewrite non-preemptible IFUNCs to IPLT functions if a non-IRELATIVE relocation is needed. (PR #133531)

[Peter Smith via llvm-branch-commits]

https://github.com/smithp35 edited 
https://github.com/llvm/llvm-project/pull/133531
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [lld] ELF: Only rewrite non-preemptible IFUNCs to IPLT functions if a non-IRELATIVE relocation is needed. (PR #133531)

[Peter Smith via llvm-branch-commits]

https://github.com/smithp35 commented:

How does this work in the non-PIE (PDE) case when we take the address of an 
ifunc and pass it to a function in a shared library, which then compares the 
argument with its own global address take of the ifunc?

For example:
shared lib
```
typedef void Fptr(void);
extern void ifn(void);

// take address of ifunc ifn defined in application
Fptr* ifp = &ifn;

// compare address of ifn we have calculated in ifp vs
// address calculated by application, passed in fp1.
int compare(Fptr* fp1) {
  return fp1 == ifp;
}
```
App
```
typedef void Fptr(void);
extern int compare(Fptr* fp1);
int val = 0;
static void impl(void) { val = 42; }
static void *resolver(void) { return impl; }
__attribute__((ifunc("resolver"))) void *ifn();

extern Fptr* fp;

int main(void) {
  return compare(fp);
}
// separate file so compiler is unaware ifn is an ifunc.
typedef void Fptr(void);
extern void ifn(void);
Fptr* fp = &ifn;
```

Right now in the application lld produces an iPLT entry for `ifn`, with `fp` 
pointing to the iPLT entry. The dynamic symbol table contains the address of 
the iPLT entry with type STT_FUNC . The shared library and the argument compare 
equal.

As I understand it, this patch will change `fp` to point directly to the result 
of the ifunc resolver. So unless we also change the value put into the dynamic 
symbol table we'll stop comparing equal. 

I don't think there's a STT_FUNC symbol we can put in the dynamic symbol table 
that holds the result of the ifunc resolver. GNU ld, puts the address of the 
resolver function with a STT_GNU_IFUNC symbol type in the dynamic symbol table. 
If that causes the dynamic loader to call the resolver and replace the value 
with the result then that would work. I haven't had time to check what glibc 
does though.

I'll put some more general comments below. Didn't want to make this one too 
long.

https://github.com/llvm/llvm-project/pull/133531
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [lld] ELF: Only rewrite non-preemptible IFUNCs to IPLT functions if a non-IRELATIVE relocation is needed. (PR #133531)

[Peter Smith via llvm-branch-commits]

smithp35 wrote:

I have some small reservations about using ifunc resolvers like this. Mostly in 
that we are using a mechanism invented for a different purpose, and relying on 
some specific linker behaviour to make this case work.

This is similar to comments made in the Discourse post 
https://discourse.llvm.org/t/rfc-structure-protection-a-family-of-uaf-mitigation-techniques/8/9
 but repeating them here as this is closest to the implementation.

As I understand it, this has a more limited and more specific use case than 
ifuncs. Traditional ifuncs which can be address taken or called, possibly in 
multiple ways, so it makes sense to use a symbol type STT_GNU_IFUNC rather than 
special relocation directives. The initializers for structure field protection 
are compiler generated, can not be legally called or address taken from user 
code, and only have one relocation type R_*_ABS64 (or 32 on a 32-bit platform). 
With an addition of a single relocation, something like R_*_ADDRINIT64 which 
would target a STT_FUNC resolver symbol. We can isolate the structure field 
initialization use case from an actual ifunc.

I guess it all comes down to whether structure field initialization needs, or 
benefits from being distinguished from an ifunc. Ifuncs seem to be quite easy 
to get wrong so being able to isolate this case has some attraction to me at 
least. It also handles the structure field that points to an ifunc relatively 
gracefully.

As you pointed out in your response, this does mean adding 2 relocations to 
every psABI that supports structure field protection rather than just one. 
Although I'd expect the alternative of having relocations that alternatively 
write "directly call" ifunc resolver or take address of function might require 
new relocations too?

https://github.com/llvm/llvm-project/pull/133531
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [lld] ELF: Only rewrite non-preemptible IFUNCs to IPLT functions if a non-IRELATIVE relocation is needed. (PR #133531)

[Peter Smith via llvm-branch-commits]

smithp35 wrote:

I had been assuming that this patch didn't change lld behaviour from the 
comment in handleNonPreemptibleIfunc()

I've had some time to build and run lld on my example with this change and I 
notice that with this change I'm seeing the dynamic symbol table use the ifunc 
resolver.
```
 7: 0021081412 IFUNC   GLOBAL DEFAULT14 ifn
```
Whereas my old lld uses
```
 7: 002108d0 0 FUNCGLOBAL DEFAULT   18 ifn
```

So as you say this will work and the comparison will go through OK. With that 
in mind this might be part of the reason why 3 tests are failing (visible in 
the CI):
  lld :: ELF/gnu-ifunc-canon.s
  lld :: ELF/ppc32-ifunc-nonpreemptible-pic.s
  lld :: ELF/ppc64-toc-relax-ifunc.s
The last one (ppc64...) looks to be crashing.

Will be good to update the comment in handleNonPreemptibleIfunc() 
https://github.com/llvm/llvm-project/blob/main/lld/ELF/Relocations.cpp#L1759 

https://github.com/llvm/llvm-project/pull/133531
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [lld] ELF: Only rewrite non-preemptible IFUNCs to IPLT functions if a non-IRELATIVE relocation is needed. (PR #133531)

[Peter Smith via llvm-branch-commits]

smithp35 wrote:

As a FYI, I'll be out of office till after Easter, so I may not be able to 
respond next week.

https://github.com/llvm/llvm-project/pull/133531
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] ELF: Add support for R_AARCH64_INST32 relocation. (PR #133534)

[Peter Smith via llvm-branch-commits]

smithp35 wrote:

Apologies for the delay in responding, just come back from vacation. Not had a 
chance to look through the Discourse posts yet, will do so this week.

While it may seem a bit premature, would you be able to open an issue in the 
AArch64 ABI (https://github.com/ARM-software/abi-aa/issues) with a request to 
add a new relocation directive, citing the Discourse posts and PRs?
 

 

https://github.com/llvm/llvm-project/pull/133534
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [lld] ELF: Add branch-to-branch optimization. (PR #138366)

[Peter Smith via llvm-branch-commits]

@@ -0,0 +1,92 @@
+//===- TargetImpl.h -*- C++ 
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLD_ELF_ARCH_TARGETIMPL_H
+#define LLD_ELF_ARCH_TARGETIMPL_H
+
+#include "InputFiles.h"
+#include "InputSection.h"
+#include "Relocations.h"
+#include "Symbols.h"
+#include "llvm/BinaryFormat/ELF.h"
+
+namespace lld {
+namespace elf {
+
+// getControlTransferAddend: If this relocation is used for control transfer
+// instructions (e.g. branch, branch-link or call) or code references (e.g.
+// virtual function pointers) and indicates an address-insignificant reference,
+// return the effective addend for the relocation, otherwise return
+// std::nullopt. The effective addend for a relocation is the addend that is
+// used to determine its branch destination.
+//
+// getBranchInfo: If a control transfer relocation referring to is+offset
+// directly transfers control to a relocated branch instruction in the 
specified
+// section, return the relocation for the branch target as well as its 
effective
+// addend (see above). Otherwise return {nullptr, 0}.
+//
+// mergeControlTransferRelocations: Given r1, a relocation for which
+// getControlTransferAddend() returned a value, and r2, a relocation returned 
by
+// getBranchInfo(), modify r1 so that it branches directly to the target of r2.
+template 
+inline void applyBranchToBranchOptImpl(
+Ctx &ctx, GetBranchInfo getBranchInfo,
+GetControlTransferAddend getControlTransferAddend,
+MergeControlTransferRelocations mergeControlTransferRelocations) {
+  // Needs to run serially because it writes to the relocations array as well 
as
+  // reading relocations of other sections.
+  for (ELFFileBase *f : ctx.objectFiles) {
+auto getRelocBranchInfo =
+[&getBranchInfo](Relocation &r,
+ uint64_t addend) -> std::pair 
{
+  auto *target = dyn_cast_or_null(r.sym);
+  // We don't allow preemptible symbols or ifuncs (may go somewhere else),
+  // absolute symbols (runtime behavior unknown), non-executable memory
+  // (ditto) or non-regular sections (no section data).
+  if (!target || target->isPreemptible || target->isGnuIFunc() ||

smithp35 wrote:

Yes, just checked and it does copy the relocation addend.

I agree that this wouldn't need a test case.

As an aside when checking where the addends were read in I ran into this bit of 
copyRelocations 
https://github.com/llvm/llvm-project/blob/main/lld/ELF/InputSection.cpp#L433 
```
  if (ctx.arg.relax && !ctx.arg.relocatable &&
  (ctx.arg.emachine == EM_RISCV || ctx.arg.emachine == EM_LOONGARCH)) {
// On LoongArch and RISC-V, relaxation might change relocations: copy
// from internal ones that are updated by relaxation.
InputSectionBase *sec = getRelocatedSection();
copyRelocations(
ctx, buf,
llvm::make_range(sec->relocations.begin(), sec->relocations.end()));
```

I think I mentioned in a previous comment that bolt uses emit-relocations so it 
may be worth following suite here when the transformation is applied.

I suspect that if bolt trusts the original relocation then in worst case the 
transformation is undone though. 

https://github.com/llvm/llvm-project/pull/138366
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [lld] ELF: Add branch-to-branch optimization. (PR #138366)

[Peter Smith via llvm-branch-commits]

@@ -975,6 +977,62 @@ void AArch64::relocateAlloc(InputSectionBase &sec, uint8_t 
*buf) const {
   }
 }
 
+static std::optional getControlTransferAddend(InputSection &is,
+Relocation &r) {
+  // Identify a control transfer relocation for the branch-to-branch
+  // optimization. A "control transfer relocation" means a B or BL
+  // target but it also includes relative vtable relocations for example.
+  //
+  // We require the relocation type to be JUMP26, CALL26 or PLT32. With a
+  // relocation type of PLT32 the value may be assumed to be used for branching
+  // directly to the symbol and the addend is only used to produce the 
relocated
+  // value (hence the effective addend is always 0). This is because if a PLT 
is
+  // needed the addend will be added to the address of the PLT, and it doesn't
+  // make sense to branch into the middle of a PLT. For example, relative 
vtable
+  // relocations use PLT32 and 0 or a positive value as the addend but still 
are
+  // used to branch to the symbol.
+  //
+  // With JUMP26 or CALL26 the only reasonable interpretation of a non-zero
+  // addend is that we are branching to symbol+addend so that becomes the
+  // effective addend.
+  if (r.type == R_AARCH64_PLT32)
+return 0;
+  if (r.type == R_AARCH64_JUMP26 || r.type == R_AARCH64_CALL26)
+return r.addend;
+  return std::nullopt;
+}
+
+static std::pair getBranchInfo(InputSection &is,
+   uint64_t offset) {
+  auto *i = std::lower_bound(
+  is.relocations.begin(), is.relocations.end(), offset,
+  [](Relocation &r, uint64_t offset) { return r.offset < offset; });
+  if (i != is.relocations.end() && i->offset == offset &&
+  i->type == R_AARCH64_JUMP26) {
+return {i, i->addend};
+  }

smithp35 wrote:

Agree that BTI instructions should be in a separate patch. It would require 
disassembling to find one so may result in longer link times. Skipping over BTI 
with direct branches could apply even when the target wasn't another branch.

https://github.com/llvm/llvm-project/pull/138366
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [lld] ELF: Add branch-to-branch optimization. (PR #138366)

[Peter Smith via llvm-branch-commits]

smithp35 wrote:

Thanks for the updates. I don't have any more comments.

https://github.com/llvm/llvm-project/pull/138366
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] ARM: Avoid using isTarget wrappers around Triple predicates (PR #144705)

[Peter Smith via llvm-branch-commits]

https://github.com/smithp35 commented:

This looks reasonable to me. I can't see a way of changing those properties on 
a subtarget level with attributes. 

Added the ARM backend maintainers to see if they have anything to add.

Would it be worth adding a comment to the function definitions in Subtarget 
https://github.com/llvm/llvm-project/blob/main/llvm/lib/Target/ARM/ARMSubtarget.h#L336

Something similar to the comment about the isCortexA5() in 
https://github.com/llvm/llvm-project/blob/main/llvm/lib/Target/ARM/ARMSubtarget.h#L285
 
```
/// These properties are per-module, please use the TargetMachine TargetTriple.
```

https://github.com/llvm/llvm-project/pull/144705
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [lld] ELF: Introduce R_AARCH64_FUNCINIT64 relocation type. (PR #133531)

[Peter Smith via llvm-branch-commits]

smithp35 wrote:

> @smithp35 @MaskRay ping.

I've got it on my list, will aim to get you some comments on this and 
https://github.com/llvm/llvm-project/pull/133534 by tomorrow. Unlikely to make 
it today.

https://github.com/llvm/llvm-project/pull/133531
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [lld] ELF: Introduce R_AARCH64_FUNCINIT64 relocation type. (PR #133531)

[Peter Smith via llvm-branch-commits]

@@ -0,0 +1,19 @@
+# REQUIRES: aarch64
+
+# RUN: llvm-mc -filetype=obj -triple=aarch64 %s -o %t.o
+# RUN: ld.lld %t.o -o %t
+# RUN: llvm-readelf -s -r %t | FileCheck %s
+# RUN: ld.lld %t.o -o %t -pie
+# RUN: llvm-readelf -s -r %t | FileCheck %s
+# RUN: not ld.lld %t.o -o %t -shared 2>&1 | FileCheck --check-prefix=ERR %s
+
+.data
+# CHECK: R_AARCH64_IRELATIVE [[FOO:[0-9a-f]*]]
+# ERR: relocation R_AARCH64_FUNCINIT64 cannot be used against preemptible 
symbol 'foo'
+.8byte foo@FUNCINIT

smithp35 wrote:

Although not this patch, MaskRay is proposing that we use a different syntax 
for relocation specifiers to avoid ambiguity with the addend: 
https://maskray.me/blog/2025-03-16-relocation-generation-in-assemblers

I'm proposing that aarch64 ELF follows this for data relocation specifiers 
(first one in 
https://github.com/ARM-software/abi-aa/pull/330/files#diff-c74a0dce6771ac7b499e84c140122aaa972bd9d63aed84863e675ecc9b4b2c32R659)
 

I'm assuming that we could migrate to this syntax at a later date if needed. 

https://github.com/llvm/llvm-project/pull/133531
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [lld] ELF: Introduce R_AARCH64_FUNCINIT64 relocation type. (PR #133531)

[Peter Smith via llvm-branch-commits]

https://github.com/smithp35 commented:

I don't have any more significant comments and no objections to the patch. 
Going back to my previous comments I was most concerned when the target was an 
ifunc symbol and that is now not supported.

https://github.com/llvm/llvm-project/pull/133531
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [lld] ELF: Introduce R_AARCH64_FUNCINIT64 relocation type. (PR #133531)

[Peter Smith via llvm-branch-commits]

https://github.com/smithp35 edited 
https://github.com/llvm/llvm-project/pull/133531
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] ELF: Introduce R_AARCH64_PATCHINST relocation type. (PR #133534)

[Peter Smith via llvm-branch-commits]

@@ -137,6 +137,7 @@ RelExpr AArch64::getRelExpr(RelType type, const Symbol &s,
   switch (type) {
   case R_AARCH64_ABS16:
   case R_AARCH64_ABS32:
+  case R_AARCH64_PATCHINST:

smithp35 wrote:

If the relocation is used as described in the RFC, with the target symbol 
absolute all looks well. If a non ABS symbol is used in a position independent 
context I think you'll get a recompile with PIC error message. If not position 
independent then I think the relocation will pass through and give a likely 
unpredictable value to patch the instruction.

Could it be worth putting some checking (likely in Relocation.cpp) to make sure 
the target symbol is SHN_ABS?

A related question is whether non-zero addends should be supported? For most 
instructions the addend doesn't make logical sense. There could be some 
instructions where the immediate field is in the right place to make it work, 
but I think these cases may not be worth supporting.

An alternative formulation for RELA is to completely ignore the symbol value 
and just use the bottom 32-bits of the addend field. That would give the object 
producer that defines the relocations more control over what is patched in. Not 
got a strong opinion on whether that is better though.

https://github.com/llvm/llvm-project/pull/133534
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] ELF: Introduce R_AARCH64_PATCHINST relocation type. (PR #133534)

[Peter Smith via llvm-branch-commits]

@@ -0,0 +1,41 @@
+# RUN: rm -rf %t && split-file %s %t
+# RUN: llvm-mc -filetype=obj -triple=aarch64 %t/use.s -o %t/use-le.o
+# RUN: llvm-mc -filetype=obj -triple=aarch64 %t/def.s -o %t/def-le.o
+
+## Deactivation symbol used without being defined: instruction emitted as 
usual.
+# RUN: ld.lld -o %t/undef-le %t/use-le.o
+# RUN: llvm-objdump -d %t/undef-le | FileCheck --check-prefix=UNDEF %s
+
+## Deactivation symbol defined: instructions overwritten with NOPs.
+# RUN: ld.lld -o %t/def-le %t/use-le.o %t/def-le.o
+# RUN: llvm-objdump -d %t/def-le | FileCheck --check-prefix=DEF %s
+
+## Behavior unchanged by endianness: relocation always written as little 
endian.
+# RUN: llvm-mc -filetype=obj -triple=aarch64_be %t/use.s -o %t/use-be.o
+# RUN: llvm-mc -filetype=obj -triple=aarch64_be %t/def.s -o %t/def-be.o
+# RUN: ld.lld -o %t/undef-be %t/use-be.o
+# RUN: llvm-objdump -d %t/undef-be | FileCheck --check-prefix=UNDEF %s
+# RUN: ld.lld -o %t/def-be %t/use-be.o %t/def-be.o
+# RUN: llvm-objdump -d %t/def-be | FileCheck --check-prefix=DEF %s
+
+#--- use.s
+.weak ds
+# This instruction has a single relocation: the DS relocation.
+# UNDEF: add x0, x1, x2
+# DEF: nop
+.reloc ., R_AARCH64_PATCHINST, ds
+add x0, x1, x2
+# This instruction has two relocations: the DS relocation and the JUMP26 to f1.
+# Make sure that the DS relocation takes precedence.
+.reloc ., R_AARCH64_PATCHINST, ds

smithp35 wrote:

Could be worth a test with emit-relocs to show both relocations coming out.

Thinking of Bolt, which relies on emit-relocs, I expect that it would just 
ignore the R_AARCH_PATCHINST relocations on their own as it wouldn't know how 
to recreate the original value [1]. It would have to discard any relocation at 
the same location. We're hoping to create a binary analysis ABI supplement soon 
to document conventions that binary analysis tools are using. First step 
https://github.com/ARM-software/abi-aa/pull/333

[1] In theory if we did want to let Bolt reverse a patch, the emit-relocs 
output could give the reverse patch. 

https://github.com/llvm/llvm-project/pull/133534
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits