Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce new relocation for landing pad #452

Open
wants to merge 3 commits into
base: complex-label-lp
Choose a base branch
from

Conversation

kito-cheng
Copy link
Collaborator

The R_RISCV_LPAD relocation can be used for PLT entry generation and also for
linker relaxation. Additionally, we defined a new mapping symbol type to help
users understand the function signature for the corresponding function.

The addend value is the label value, and it will point to the mapping symbol
placed at the beginning of the function.

e.g.

foo:         # void foo(void)
$sFvvE:
    lpad 123 # R_RISCV_LPAD $sFvvE + 123

We propose two linker relaxations for the landing pad. The first is removing
the entire landing pad, which can be used when symbols have local visibility,
and the address is not taken by any other reference. The second is a landing
pad scheme conversion, designed for backward compatibility (or as a workaround)
for legacy programs that may use functions without declarations.


NOTE: This is based on #434

@kito-cheng
Copy link
Collaborator Author

cc @deepak0414 @ved-rivos @mylai-mtk

riscv-elf.adoc Outdated Show resolved Hide resolved
riscv-elf.adoc Outdated Show resolved Hide resolved
Copy link
Contributor

@aswaterman aswaterman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this reloc also provides enough info for the linker to fix up direct jumps to skip over landing pads when the landing pad was not relaxed.

Consider I have call foo, and the linker sees that the target of that call still has an R_RISCV_LPAD reloc (i.e. not relaxed). The linker should be able to adjust call foo to call foo+4. This should work whether the target is the PLT or the real function.

riscv-elf.adoc Outdated Show resolved Hide resolved
riscv-elf.adoc Outdated Show resolved Hide resolved
riscv-elf.adoc Outdated Show resolved Hide resolved
riscv-elf.adoc Show resolved Hide resolved
riscv-elf.adoc Outdated Show resolved Hide resolved
riscv-elf.adoc Outdated
Comment on lines 2333 to 2338
Condition:: The associated function of this lpad must have local visibility, and
it must not be referenced by any relocation other than `R_RISCV_CALL` and
`R_RISCV_CALL_PLT`.
This relaxation can also be performed when the function has global visibility,
if the symbol does not have a corresponding PLT entry and is not referenced by
the GOT or by any relocation other than `R_RISCV_CALL` and `R_RISCV_CALL_PLT`.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can explain the behavior more clearly if we avoid mentioning symbol visibility. The only important thing here is whether or not a symbol is visible to other ELF modules, i.e. whether or not the symbol is in the dynamic symbol table. Symbol visibility is just one way to control it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good suggestion, applied 02546de :)

riscv-elf.adoc Outdated
@@ -548,7 +548,9 @@ Description:: Additional information about the relocation
<| S - P
.2+| 65 .2+| TLSDESC_CALL .2+| Static | .2+| Annotate call to TLS descriptor resolver function, `%tlsdesc_call(address of %tlsdesc_hi)`, for relaxation purposes only
<|
.2+| 66-190 .2+| *Reserved* .2+| - | .2+| Reserved for future standard use
.2+| 66 .2+| LPAD .2+| Static | .2+| Annotates the landing pad instruction inserted at the beginning of the function. The addend indicates the label value of the landing pad, and the symbol value is the address of the mapping symbol for the function signature, which will have the same address as the function.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the "label value of the landing pad?

riscv-elf.adoc Outdated
Comment on lines 2336 to 2338
This relaxation can also be performed when the function has global visibility,
if the symbol does not have a corresponding PLT entry and is not referenced by
the GOT or by any relocation other than `R_RISCV_CALL` and `R_RISCV_CALL_PLT`.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How should we find a function symbol for a given R_RISCV_LPAD relocation? Should we just look for a function symbol at the same location as the relocation refers to?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right now, this design requires scanning through the symbol table and relocation table once to figure out which symbols have landing pads. I had previously thought about creating a new section to handle this, but I realized that when dealing with linker relaxation, the best way is still to use relocations to mark them. This approach also avoids introducing a new section format.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right now, this design requires scanning through the symbol table and relocation table once to figure out which symbols have landing pads.

Does this mean that O(R + F) time complexity and O(R) space complexity, where R is number of LPAD relocations and F is number of global function symbols in the same section, to build a map from function symbols to lpad labels is expected? This map is needed to synthesize PLT.

Note: The complexities come from the following algorithm:

HashMap<Address, LabelLabel> LpadRelocMap;
HashMap<FunctionSymbol, LpadLabel> LpadLabelMap;
for (auto R: LpadRelocations)
  LpadRelocMap.insert(R.Address, R.addend);

for (auto S: FunctionSymbols)
  if (S.Address in LpadRelocMap) lpadLabelMap.insert(S, LpadRelocMap[S.Address]);

Description:: This relaxation type allows an `lpad` instruction to be relaxed
into `lpad 0`, which is a universal landing pad that ignores the label value
comparison. This relaxation is used when the label value is not computed
correctly.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what would be the cases where a label may be computed incorrectly.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some legacy programs don’t properly declare function prototypes before calling them. In these cases, the compiler will infer a function prototype based on the language standards, but it often ends up being incorrect. One common example is dhrystone[1]. In most versions you find online, Func_2 isn’t declared before it’s called, so the compiler will assume the prototype is int Func_2(char*, char*), but the correct prototype is actually void Func_2(char[31], char[31]).

[1] https://github.com/sifive/benchmark-dhrystone/blob/master/dhry_1.c#L164

Another common potential issue in C is with qsort. Function pointers can be compatible but not perfectly match the expected type. For example, here’s how qsort is declared:

void qsort(void* ptr, size_t count, size_t size, int (*comp)(const void*, const void*));

But in practice, you can pass in a compatible, but not exactly matching, type for the comparison function, and it works in most cases:

#include <stdlib.h>

int compare(int *a, int *b)  // The signature isn’t int (*)(const void*, const void*)
{
    return *(int *)a - *(int *)b;
}

void foo(int *x, size_t count, size_t size)
{
    qsort(x, count, size, compare);  // But in practice, this works fine
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But how is the linker expected to know the incorrectness so it can perform this relaxation?

The Zicfilp mechanism is employed when issuing an indirect call through function pointers, and when calling functions through PLT:

In the first case (indirect calls through pointers), to know that an lpad insn needs to be relaxed to lpad 0 due to the original label being incorrect, the linker would need to know where the pointer points to, so the caller's label (the "correct" one) can be checked against the callee's label (the lpad insn). I'm not sure if this is the scenario you're targeting, but if it is, I think this (knowing where the pointer points to, or knowing where the call would come from) is an expectation too high for linkers. Besides, in this scenario, I would also wonder how linkers are expected to retrieve the caller's label (the "correct" one) so it can make the comparison?

In the second case (calls through PLT), the indirect call happens in the PLT, which is generated by linkers. The label which linkers use to generate PLT would come from the addend of the LPAD relocation, which should contain the same label as the one in its referenced lpad insn, so there would be no chance of mismatch and thus incorrectness identifiable by linkers.

The above is my guess and understanding of the intended usage of this relaxation. If we're not on the same page, please do let me know.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Linker never know (or not always know), and also that's not the right layer to analysis (or guess:P ), so I expect that relaxation should only enabled when user pass something like -z force-simple-landing-pad-scheme to linker.

riscv-elf.adoc Outdated
@@ -1582,6 +1584,7 @@ A number of symbols, named mapping symbols, describe the boundaries.
| $x.<any>
| $x<ISA> .2+| Start of a sequence of instructions with <ISA> extension.
| $x<ISA>.<any>
| $s<function-signature-string> | Marker for the landing pad instruction. This should only be used with the function signature-based scheme and should be placed only at the beginning of the function.
Copy link

@mylai-mtk mylai-mtk Oct 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't quite get the purpose of this mapping symbol: It looks like the only reference to these symbols come from the LPAD relocation, but for what? The LPAD relocation already have the 20-bit label stored in its addend, and its link to this %s mapping symbol provides only the additional information of the signature string, which is not needed (as of now) to link the object files.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

riscv/riscv-cfi#151 (comment)

It's kinda debugging propose only, so it safe to strip like all other mapping symbols

Copy link

@mylai-mtk mylai-mtk Oct 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the purpose is to display function signatures when disassembling, this mechanism seems a bit incomplete (?) I suppose since the relocation is a static one, it would not stay in the binary after static linking, thus if a user disassembles a linked ELF, it's still the label numbers instead of signatures that get displayed?

Update: Assuming it's relying on the mapping symbol having the same address as the lpad insn to associate an lpad insn to a function signature (so that the signature can be displayed when disassembling a linked binary), why do relocations refer to these symbols?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the purpose of the mapping symbol is to provide debug/disassembling info, I think after the introduction of the .riscv.lpadinfo section, this purpose can be better served by the new section, since it contains the same information, and has a more size-compact format than the symbol table.

@mylai-mtk
Copy link

Q: Does the LPAD relocation and/or the $s<func-sig> mapping symbol serve the purpose of providing labels when generating PLT? If so, how is it expected to extract the label for a given function symbol from these new entities?

@rui314
Copy link
Collaborator

rui314 commented Oct 18, 2024

I don't think I fully understand this proposal, but from the linker's perspective, I believe we just need the following:

  • A R_RISCV_LPAD relocation, whose r_offset is at each removable lpad instruction location, and r_sym refers to a function symbol
  • If the function's address was not taken (i.e. the function symbol was not referenced by anything but R_RISCV_CALL or R_RISCV_CALL_PLT) and the function symbol is not in the dynamic symbol, the linker removes the lpad instruction.

I don't think we need a mapping symbol for the linker to remove a lpad instruction.

@kito-cheng
Copy link
Collaborator Author

@rui314

I don't think I fully understand this proposal, but from the linker's perspective, I believe we just need the following:

A R_RISCV_LPAD relocation, whose r_offset is at each removable lpad instruction location, and r_sym refers to a function symbol
If the function's address was not taken (i.e. the function symbol was not referenced by anything but R_RISCV_CALL or R_RISCV_CALL_PLT) and the function symbol is not in the dynamic symbol, the linker removes the lpad instruction.
I don't think we need a mapping symbol for the linker to remove a lpad instruction.

The design of the mapping symbol is meant to make debugging easier for users, so it’s actually optional. This was discussed in the CFI spec issue [1]. If we set aside that purpose, I also think it’s a better approach to have r_sym point to the function symbol.

As for using the addend to record the landing pad value, it helps with PLT generation. The linker can get the value by scanning the instruction at that address, but encoding it directly in the addend avoids needing to read the instruction during relocation.

[1] riscv/riscv-cfi#151 (comment)

riscv-elf.adoc Outdated
@@ -548,7 +548,9 @@ Description:: Additional information about the relocation
<| S - P
.2+| 65 .2+| TLSDESC_CALL .2+| Static | .2+| Annotate call to TLS descriptor resolver function, `%tlsdesc_call(address of %tlsdesc_hi)`, for relaxation purposes only
<|
.2+| 66-190 .2+| *Reserved* .2+| - | .2+| Reserved for future standard use
.2+| 66 .2+| LPAD .2+| Static | .2+| Annotates the landing pad instruction inserted at the beginning of the function. The addend indicates the label value of the landing pad, and the symbol value is the address of the mapping symbol for the function signature, which will have the same address as the function.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this relocation only for the func-sig scheme? Based on its description, it looks like so, but the following LPAD relaxation that removes the lpad insn seems also applicable to the unlabeled scheme.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That should also work for unlabeled scheme as well, let me think how to make it clearly.

@mylai-mtk
Copy link

mylai-mtk commented Oct 23, 2024

@kito-cheng

As for using the addend to record the landing pad value, it helps with PLT generation. The linker can get the value by scanning the instruction at that address, but encoding it directly in the addend avoids needing to read the instruction during relocation.

I think this doesn't provide the lpad label we need for PLT generation? For a call target to be generated in the PLT, the target should reside in a shared library or somewhere that the linker cannot determine during static-time linking. When the target resides in a shared library, it does not have the LPAD relocation associated with it, since the relocation is a static relocation and should be consumed by the static linker that created the shared library?

@kito-cheng
Copy link
Collaborator Author

@mylai-mtk

I think this doesn't provide the lpad label we need for PLT generation? For a call target to be generated in the PLT, the target should reside in a shared library or somewhere that the linker cannot determine during static-time linking. When the target resides in a shared library, it does not have the LPAD relocation associated with it, since the relocation is a static relocation and should be consumed by the static linker that created the shared library?

Hmmmmmmmmmmmmmm, yeah, do you have any better idea that creating a new section to recording the label value for those undefined symbols?

@mylai-mtk
Copy link

@kito-cheng

Hmmmmmmmmmmmmmm, yeah, do you have any better idea that creating a new section to recording the label value for those undefined symbols?

Nope. After skimming through existing sections, I don't have ideas better than creating a new section.

My previous prototype of generating extra symbols for every function may work, but there's too many drawbacks with this approach, including bloated symbol table size and slowed down symbol resolution performance, so I would not recommend this approach seriously.

@kito-cheng
Copy link
Collaborator Author

@mylai-mtk added new section. and also new asm directive riscv-non-isa/riscv-asm-manual#113

riscv-elf.adoc Outdated Show resolved Hide resolved
riscv-elf.adoc Show resolved Hide resolved
riscv-elf.adoc Show resolved Hide resolved
riscv-elf.adoc Outdated
NOTE: Using same encoding as mapping symbol aims to reduce the size of the
string table

Every symbol with global or weak bind must has a corresponding entry in this

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you mean "Every function symbol with global or weak"?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yeah, should just restrict to symbol with function type

Copy link

@mylai-mtk mylai-mtk Nov 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, we should restrict this to global or pointer-taken symbols, i.e. function symbols with an lpad instruction after relaxation

Update: No need to have the above requirement, since trimming down the size of the .riscv.lpadinfo section is just nice-to-have but not must-have. Having unneeded entries is not ideal, but it would not affect the correctness, so it's not necessary to mandate this in a specification.

section, the `lpi_name` field must be the same as the symbol name string table
index.

This section can be discard after static linking stage.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since you mentioned that "Every symbol with global or weak bind must has a corresponding entry", I think it implies that the lpad labels are provided by the object file that defines the function, right? If this is the case, we can't discard this section after static linking when creating a shared library, since library users would expect to find lpad labels later when linking against this share library.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can still know the signature/landing pad label value when we reference to a symbol which is undefined yet, because we always need declare the prototype in the source code.

"Every symbol with global or weak bind must has a corresponding entry" -> we didn't exclude the undefined symbol, so we can link to the shared library without that info

Copy link

@mylai-mtk mylai-mtk Nov 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If labels can be provided by the object that uses but doesn't define the function, why require labels to be there in the defining object ("Every symbol with global or weak bind must has a corresponding entry")? For the sake of checking if the use-site and define-site agree on the same lpad label?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's still better to put the .riscv.lpadinfo section in shared libraries. I can come up with two possible cases in which the C language prototype is hard to obtain:

Case 1: When writing assembly by hand, the pseudo call or jump instruction is used. These instructions would cause the CALL_PLT relocation to be generated in the resulting relocatable, which would lead to PLT entries being generated by the static linker.

Case 2: Compiler inserts calls to builtins or instrumentations. These extra function calls are often not inserted at the C source level, but at the compiler IR level. This makes knowing the C prototype of the called target hard as there would not be a C language data structure to represent the called target in the AST.

In these cases, it's not easy to obtain the C language prototype, as the only assumption in these cases is that there would be a defined symbol to which the linker can resolve the called target. Considering these cases, I would still prefer to have the shared libraries provide the lpad labels for functions defined by them.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with you, so...I just say can be not must be here, that make it strip-able, but it's also legal to leave here

Copy link

@mylai-mtk mylai-mtk Dec 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm, if this is the intention, I would suggest you make it clear that it means the section is strip-able, and does not mean that the section can be discarded under all circumstances after static linkage. I believe if a user strips his binary, he should know the consequence, so marking a section that is only strip-able under certain situations strip-able is acceptable.

My concern is that if this sentence is considered by a linker implementation to be the spec of linker behavior that uniformly allows the section to be dropped after static linkage, the spec is flawed, since when producing shared libraries, the section cannot be dropped or otherwise the library could risk linkage failure to relocatables.

riscv-elf.adoc Outdated Show resolved Hide resolved
riscv-elf.adoc Outdated
Comment on lines 1607 to 1611
The string hold by `lpi_signature` field is the function signature string, which
is encoded as same as the mapping symbol of the function signature.

NOTE: Using same encoding as mapping symbol aims to reduce the size of the
string table

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This means the signatures stored start with $ and is in the format of $x<function-signature-string>? (Note the additional x there)

If the goal is to save bytes in the string table, I think we can always use Symbol($x<function-signature-string>).st_name + 2 to specify the signature string to keep the referred signature precise without the $x prefix?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Drop mapping symbol so this paragraph will be removed

riscv-elf.adoc Outdated
@@ -1582,6 +1584,7 @@ A number of symbols, named mapping symbols, describe the boundaries.
| $x.<any>
| $x<ISA> .2+| Start of a sequence of instructions with <ISA> extension.
| $x<ISA>.<any>
| $s<function-signature-string> | Marker for the landing pad instruction. This should only be used with the function signature-based scheme and should be placed only at the beginning of the function.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the purpose of the mapping symbol is to provide debug/disassembling info, I think after the introduction of the .riscv.lpadinfo section, this purpose can be better served by the new section, since it contains the same information, and has a more size-compact format than the symbol table.

riscv-elf.adoc Outdated
@@ -548,7 +548,9 @@ Description:: Additional information about the relocation
<| S - P
.2+| 65 .2+| TLSDESC_CALL .2+| Static | .2+| Annotate call to TLS descriptor resolver function, `%tlsdesc_call(address of %tlsdesc_hi)`, for relaxation purposes only
<|
.2+| 66-190 .2+| *Reserved* .2+| - | .2+| Reserved for future standard use
.2+| 66 .2+| LPAD .2+| Static | .2+| Annotates the landing pad instruction inserted at the beginning of the function. The addend indicates the label value of the landing pad, and the symbol value is the address of the mapping symbol for the function signature, which will have the same address as the function.
Copy link

@mylai-mtk mylai-mtk Nov 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this LPAD relocation intend to serve the "real" purpose of a relocation? That is, ask linkers to fill-in some value (in this case, the label of the lpad instructions) to some offset at link time.

When prototyping this relocation in LLVM, it appears to me that the LLVM backend assumes places to be relocated have a placeholder value 0 encoded, so to emit the LPAD relocations, 0s would have to be encoded at the label locations in relocatable files. This can of course be changed to encode the correct label along with relocation emitted, but I want to ask if this change is needed, or I can rely on linkers to fill-in the correct labels when relocating at static time.

riscv-elf.adoc Outdated Show resolved Hide resolved
riscv-elf.adoc Outdated Show resolved Hide resolved
riscv-elf.adoc Outdated
Comment on lines 1580 to 1582
Landing pad information section is a section that contains the nessary information
for generating function signature based landing pad PLT, this section also may
exsiting when the unlabeled landing pad scheme is used.
Copy link

@mylai-mtk mylai-mtk Dec 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change to: The landing pad information section is a section that contains the information to generate PLT with landing pads. The static linker is required to use the landing pad values provided by this section when generating lpad instructions for functions listed in this section, unless otherwise specified by the user (e.g. through command line options) or the unlabeled CFI scheme is selected.

This change allows us to:

  • Avoid limiting the use of this section to the function-signature scheme
  • Be clear that when lpad labels can be inferred from other information sources contained in the source object, the riscv.lpadinfo section would take precedence, unless the linker user commands otherwise clearly (e.g. through the -z force-zicfilp=unlabeled command line option) or the unlabeled CFI scheme is selected (the unlabeled scheme implies an optimized PLT entry sequence, thus 0 must be used for lpad labels.)

Update: Exclude the unlabeled CFI scheme from adopting .riscv.lpadinfo section contents, since it implies an optimized PLT entry sequence, thus 0 must be used for lpad labels.

riscv-elf.adoc Outdated
```
typedef struct
{
Elf32_Word lpi_name; /* Symbol name (string tbl index) */
Copy link

@mylai-mtk mylai-mtk Dec 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just curious: Why isn't this the index of symbol in the symbol table?

The symbol table index provides a more definite reference to the symbol, considering the case of repeated symbol names (though this may not really happen with function symbols as its an error to globally define a function more than once.)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've come up with a case that would cause the current format to fail:

// public_foo.c
void foo() {}

// private_foo_1.c
static int foo(int i) { return i; }
void *get_foo_1() { return foo; }

// private_foo_2.c
static char foo(char c) { return c; }
void *get_foo_2() { return foo; }

Compile them to a shared library, and you get 1 GLOBAL FUNC and 2 LOCAL FUNC foo symbols, and the signature of these 3 foos are different. With the current format, the lpad info entries of these 3 symbols cannot be distinguished, and this would cause problem for the linker.

To support showing lpi_info contents when disassembling, none of the 3 lpad info entries can be omitted, so to distinguish them, symbol table indices should be used in place of string table indices to refer to symbols, and as a consequence, lpi_name should be renamed to lpi_sym or lpi_func (the field does not represent "name" anymore).

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TL;DR: I will update to the symbol index in next update.

And let me tell the story why I use the symbol name before: In the earlier version, we have mapping symbol at the beginning of the function, so local function could use that only, but we gonna to remove the mapping symbol, so that means local symbol may also rely this mechanism to record the signature.

Recording the symbol name is fine before since global symbol has unique symbol name (in theory), and local symbol may have duplicated symbol name, but it's not a problem before.

The concern for using the symbol index is: .symtab will gone after strip, so the index is kinda become meaningless, and we have only index for .dynsym, but .dynsym only contain exported global symbol only, however this issue can be resolve if we say the .riscv.lpadinfo should be stripped if .symtab got stripped.

So the conclusion is: I think we should use symbol index here, rather than symbol name as your suggestion.

Copy link

@mylai-mtk mylai-mtk Dec 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your explanation, and sorry for ruining your original design about mapping symbol 😥

The main reason I was against the mapping symbol approach was that it's intended usage was obscure. It only had a reference from the LPAD relocation that is intended to provide the disassembling string for the lpad label, but considering that LPAD relocations would be gone after static linking, this solution looks incomplete (it only provides the disassembling string for relocatables, but not for executables and shared libraries), so it didn't convince me that we should adopt the mapping symbol approach.

But if how the mapping symbols were associated to the lpad insns is clarified and unified by dropping the reference in the LPAD relocation, and solely relies on the mapping symbols having the st_value that points to the lpad insns, I think using mapping symbols to provide the disassembling strings actually has its advantages (and is better than using .riscv.lpadinfo):

  • It allows decorating lpad insns not only at the beginning of function symbols, but also in the middle of function bodies
  • As a kind of debug info, it's automatically stripped away when the the binary is stripped
  • Symbol handling in compiler backends and linkers has extensive infrastructure support. Not much work would need to be carried out to have the mapping symbols properly emitted
  • If the approach is adopted, we can remove the lpi_info field from the .riscv.lpadinfo section, making the section simpler and probably more compact and concise than the current format
    • Also, separating the optional debug info from the necessary .riscv.lpadinfo simplifies the treatment of the .riscv.lpadinfo section in toolchain (it would be easier to determine if the section needs to kept or can be discarded, and relevant data structures would be easier to maintain)

The only disadvantage (I can come up with) of the updated mapping symbol approach would be a bloated symbol table, but symbol tables are strippable in the final executables/shared libraries, so it does not matter much in production after all.

Saying so much, would you mind to clarify how the mapping symbol approach was supposed to work, and let's reconsider it (and the format .riscv.lpadinfo)?

Thank you 🙇‍♂️

@mylai-mtk
Copy link

mylai-mtk commented Dec 24, 2024

Is it possible that we design this spec in terms of just labeled and unlabeled schemes, instead of the more constraining func-sig and unlabeled schemes? (Same goes for the PLT draft spec)

The motivation is to increase the versatility of these specs. This idea is based on the observation (and "feeling" during my prototyping) that nearly all components of these specs for the func-sig scheme (including the PLT form, relocations, relaxations, .riscv.lpadinfo section content, mapping symbol, and the %lpad_label("func-sig") assembly operand modifier) are designed with just the assumption that the lpad values can be non-zero, and they come from strings. This implies that the specification of "func-sig" can actually be removed to make these specs more versatile to support all schemes that generates lpad values from strings, which is flexible enough to encode almost all data formats and therefore, most of future possible schemes.

@mylai-mtk
Copy link

Proposal: Should we consider relaxation that removes (at best effort) lpad insns if the resulting GNU AND features does not contain CFI_LP flags?

but the linker should pad nop instruction to the same length of the original
instruction sequence.

Condition:: This relaxation can only be applied if the symbol is **NOT**

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Current relocation refers to the mapping symbol, but I guess this condition is not made on the mapping symbol, but on the function symbol?

@mylai-mtk
Copy link

mylai-mtk commented Jan 9, 2025

Hi, I have implemented a prototype of this draft in the LLVM toolchain. Since there are some problems if the prototype strictly follows the draft spec, I had modified the spec and here I list my (major) modifications and the reasons:

-- Comment edited: I have uploaded my prototype at https://github.com/mylai-mtk/llvm-project/releases/tag/zicfilp-lpadinfo-20250110, and annotated the spec modifications with code implementation references. Feel free to check it out for more details.


  • Current draft: (None)
  • Modified spec: If the LPAD relocation refers to a symbol, the addend is 0, the func-sig scheme is adopted, and the referred symbol has a non-zero lpad value listed in .riscv.lpadinfo, the lpad insn referred to by the LPAD relocation is updated to use the referred symbol's non-zero lpad value from .riscv.lpadinfo. The static linker can emit a warning if the LPAD relocation's referred symbol has a zero lpad value listed in .riscv.lpadinfo when the LPAD relocation refers to a symbol, the addend is 0, and the func-sig scheme is adopted
  • Reason: This relaxation enables functions implemented in assembly but called from C to conveniently use lpad 0 as its lpad value, but still gets a non-zero lpad value, e.g.
foo:
  lpad 0
  ...
// This line causes compiler to emit correct label for `foo` in `.riscv.lpadinfo`,
// which can be used to fix the lpad insn for `foo`.
extern int foo(void);

  • Current draft: The array element structure of .riscv.lpadinfo section is
typedef struct {
  ElfN_Word lpi_name;  // Symbol name (string table index)
  ElfN_Word lpi_sig;   // Signature for the symbol (string table index)
  ElfN_Word lpi_value; // Landing pad value for the symbol
} ElfN_Lpadinfo;
  • Modified spec: The array element structure of .riscv.lpadinfo section is
typedef struct {
  ElfN_Word lpi_sym;   // Function symbol (`.symtab`/`.dynsym` table index)
  ElfN_Word lpi_value; // Landing pad value for the symbol
} ElfN_Lpadinfo;

The lpi_sym fields refer to symbols in .dynsym for shared libraries; for other ELF files, they refer to .symtab

  • Reason:

    • lpi_name is changed to lpi_sym, since intuitively, it's more precise to refer to a symbol using its symbol table index rather than its name, but perhaps more importantly, this allows static linkers to avoid the complex process of resolving names to symbols, especially in the case of versioned symbols
    • lpi_sig is removed, since @kito-cheng provided/explained a better idea to store the information using mapping symbols. By removing lpi_sig from .riscv.lpadinfo, it's also now clear that the section is needed only before static linkage, and can be discarded afterwards
      • Note that shared libraries are always possible to be fed to static linkers, so strictly speaking it's not recommended to throw away their .riscv.lpadinfo
    • lpi_sym refers to .dynsym/.symtab for shared libraries/other files, since it's more natural for the static linker to look at these different symbol tables for these file kinds. For example, llvm-lld does not parse .symtab for shared libraries and instead uses the same code/data fields to parse .dynsym.
  • Prototype ref:

    • For .riscv.lpadinfo section emission of relocatables, look at commit mylai-mtk/llvm-project@29d7691. Start from RISCVAsmBackend::getLpadinfoSectionContent()
    • For .riscv.lpadinfo section emission of shared libraries, look at commit mylai-mtk/llvm-project@cde0a27. Start from RISCVLpadinfoSection::writeTo()

  • Current draft: Every global/weak function symbol must have an entry in .riscv.lpadinfo
  • Modified spec: Every global/weak function symbol, and undefined global/weak symbol that are expected to be a function, must have an entry in .riscv.lpadinfo
  • Reason: Undefined global symbols that are expected to be a function may provide a non-zero lpad value that can be used to overwrite a zero lpad value. Undefined weak symbols that are expected to be a function may have a PLT entry, so their lpad values are needed
  • Prototype ref: Look at commit mylai-mtk/llvm-project@29d7691. More specifically, RISCVAsmPrinter::emitLpadInfos() records lpad infos for all functions (both definitions and declarations), and RISCVAsmBackend::getLpadinfoSectionContent() writes out the final .riscv.lpadinfo section based on those records

  • Current draft: Static linkers should emit error if objects with same symbol but different lpad values are beging merged
  • Modified spec: Static linkers should merge lpad values from objects using the following logic: If a resolved symbol has a zero and a non-zero lpad value coming from objects that defines, either globally or weakly, the symbol, the non-zero lpad value is adopted as the "definitive lpad value" for the symbol. Otherwise, if there are only zero lpad values for the symbol in these objects, the definitive lpad value for the symbol is 0. If the symbol defining objects provides more than 1 non-zero lpad values for a resolved symbol, an error should be emitted. Static linkers should also emit an error if for a symbol, the lpad values coming from objects that just references it (not defining it) conflicts with each other, i.e. they are different. Note that the merging between zero and non-zero lpad values does not apply to the lpad values coming from objects that just references the symbol, since these references are expected to be generated through C declarations, so only non-zero lpad values are expected. The lpad value provided by objects that just references a symbol is termed "referencing lpad value" for the symbol. The final lpad value is merged from definitive and referencing lpad values: If the definitive lpad value exists and is non-zero, the final lpad value is definitive lpad value. Otherwise, if the referencing lpad value exists, the final lpad value is the referencing lpad value. Otherwise, if the definitive lpad values exists but is 0, the final lpad value is 0. Note that the final lpad value is unknown if both definitive and referencing lpad values do not exist.
  • Reason: This lpad value merging logic aims to allow the convenience of lpad 0 to be supplemented with the robustness of lpad values coming from C declarations, while still guarding against lpad value conflictions. The separation of definitive and referencing lpad values is based on the observation that build system test programs (e.g. simple C program from CMake that tests whether a function is available) often use incorrect function signatures. These incorrect function signatures would trigger lpad value confliction if they were not considered separately from definitive lpad values. By separating definitive and referencing lpad values and giving definitive lpad values a higher priority, we can safely pass these incorrect programs while preventing their incorrectness from contaminating the final lpad values to the largest extent.
  • Prototype ref:

-- Comment edited: Lpad merging logic wording is tweaked to clarify the algorithm.


  • Current draft: The .riscv.lpadinfo section may exist when the unlabeled scheme is adopted
  • Modified spec: The .riscv.lpadinfo section may not exist, or is ignored, when the unlabeled scheme is adopted
  • Reason: The modified .riscv.lpadinfo section is purely for the purpose of providing lpad values for a labeled scheme. It does not make sense to handle this section when the unlabeled scheme is adopted

  • Current draft: The LPAD relocation refers to the mapping symbol, which has the st_value that points to the lpad insn
  • Modified spec: The LPAD relocation optionally refers to a function symbol, which has the st_value that points to the lpad insn, when the lpad value is 0
  • Reason: This facilitates the new relaxation that tightens lpad 0 to lpad non-0 with information from .riscv.lpadinfo. The referred to function symbol is used to lookup information in .riscv.lpadinfo
  • Prototype ref: Look at https://github.com/mylai-mtk/llvm-project/blob/13515c585ae1ddbc4956e884e5fc510a7c6ffa64/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCExpr.cpp#L272 to see that when the lpad value is 0 and a function symbol that points to the lpad insn can be obtained, that function symbol is inserted into the data structure that would later be converted to a relocation

-- Commented edited: Add words to indicate that the function symbol may exist only when the lpad value is 0

@kito-cheng
Copy link
Collaborator Author

Is it possible that we design this spec in terms of just labeled and unlabeled schemes, instead of the more constraining func-sig and unlabeled schemes? (Same goes for the PLT draft spec)

I consider that before but that mean we need to introduce another attribute or property to record which scheme are used - so I think let keep allocate dedicate bit for function signature - but I do agree we could relax the wording a little bit to make this spec to introduce new labeling scheme without too much modifier in future :)

@kito-cheng
Copy link
Collaborator Author

I am putting all modifier together and plan to update this tomorrow (if no accident or other urgent thing pop up), also I will put your (@mylai-mtk) name in the commit as coauthor :)

@kito-cheng
Copy link
Collaborator Author

Current draft: Static linkers should emit error if objects with same symbol but different lpad values are beging merged
Modified spec: Static linkers should merge lpad values from objects using the following logic: If a resolved symbol has a zero and a non-zero lpad value coming from objects that defines, both globally and weakly, the symbol, the non-zero lpad value is adopted as the "definitive lpad value" for the symbol. Otherwise, the definitive lpad value for the symbol is 0. If the symbol defining objects provides more than 1 non-zero lpad values for a resolved symbol, an error should be emitted. Static linkers should also emit an error if for a symbol, the lpad values ("referencing lpad values") coming from objects that just references it (not defining it) conflicts with each other, i.e. they are different. Note that the merging between zero and non-zero lpad values does not apply to the lpad values coming from objects that just references the symbol, since these references are expected to be generated through C declarations, so only non-zero lpad values are expected. The lpad value provided by objects that just references a symbol is termed "referencing lpad values" for the symbol. The final lpad value is merged from definitive and referencing lpad values: If the definitive lpad value exists and is non-zero, the final lpad value is definitive lpad value. Otherwise, if the referencing lpad value exists, the final lpad value is the referencing lpad value. Otherwise, if the definitive lpad values exists but is 0, the final lpad value is 0. Note that the final lpad value is unknown if both definitive and referencing lpad values do not exist.
Reason: This lpad value merging logic aims to allow the convenience of lpad 0 to be supplemented with the robustness of lpad values coming from C declarations, while still guarding against lpad value conflictions. The separation of definitive and referencing lpad values is based on the observation that build system test programs (e.g. simple C program from CMake that tests whether a function is available) often use incorrect function signatures. These incorrect function signatures would trigger lpad value confliction if they were not considered separatively from definitive lpad values. By separating definitive and referencing lpad values and giving definitive lpad values higher priority, we can safely pass these incorrect programs while preventing their incorrectness from contaminating the final lpad values to the largest extent.

My first impression is why we need merge logic, but after reading your reason I think it's reasonable - although I real want to say legacy code just go to the hell - but the reality is that we still live in a hell that is everywhere (Look at this fantastic and amazing dhrystone!)

@mylai-mtk
Copy link

I had uploaded my prototype (URLs provided in the spec modification discussion) so more details can be observed. The prototype has passed some C/C++ language test suites.


I consider that before but that mean we need to introduce another attribute or property to record which scheme are used - so I think let keep allocate dedicate bit for function signature - but I do agree we could relax the wording a little bit to make this spec to introduce new labeling scheme without too much modifier in future :)

👍

I am putting all modifier together and plan to update this tomorrow (if no accident or other urgent thing pop up), also I will put your (@mylai-mtk) name in the commit as coauthor :)

Thank you

kito-cheng and others added 3 commits January 10, 2025 14:26
The purpose of the landing pad relocation is to enable linker relaxation,
allowing the linker to remove the landing pad instruction if possible.

Co-authored-by: Ming-yi Lai <[email protected]>
The mapping symbol for the landing pad (`$s`) provides additional information
for the labeled landing pad scheme. This mapping symbol may be generated when
setting up a landing pad value (e.g., `auipc t2, %lpad_hash("FvvE")` will
generate `$sFvvE`) or when emitting a landing pad instruction (e.g.,
`lpad %lpad_hash("FvvE")` will generate `$sFvvE`).

Co-authored-by: Ming-yi Lai <[email protected]>
The `.riscv.lpadinfo` section is used for PLT generation and for cross-verifying
the landing pad values between different objects and shared libraries.

```
typedef struct
{
  ElfNN_Word    lpi_sym;                 /* Symbol index */
  ElfNN_Word    lpi_value;               /* Landing pad value for the symbol */
} ElfNN_Lpadinfo;
```

Additionally, a merge policy is defined to handle mismatched landing pad values
between objects and shared libraries.

Co-authored-by: Ming-yi Lai <[email protected]>
@kito-cheng
Copy link
Collaborator Author

Changes:

  • LPAD relocation no longer require addend and symbol.
  • Mapping symbol has extend to used in landing pad setup instruciton as well. e.g. auipc t2, %lpad_hash("FvvE") will having a $sFvvE
  • Drop lpi_sig from ElfXX_Lpadinfo
  • Rename lpi_name to lpi_sym, and it means symbol index rather than string index now.
  • Re-org the commits into 3 pieces: LPAD relocation and relaxation, .riscv.lpadinfo section and mapping symbol

NOTE:
I take most comments from @mylai-mtk except this:

Current draft: (None)
Modified spec: If the LPAD relocation refers to a symbol, the addend is 0, the func-sig scheme is adopted, and the referred symbol has a non-zero lpad value listed in .riscv.lpadinfo, the lpad insn referred to by the LPAD relocation is updated to use the referred symbol's non-zero lpad value from .riscv.lpadinfo. The static linker can emit a warning if the LPAD relocation's referred symbol has a zero lpad value listed in .riscv.lpadinfo when the LPAD relocation refers to a symbol, the addend is 0, and the func-sig scheme is adopted
Reason: This relaxation enables functions implemented in assembly but called from C to conveniently use lpad 0 as its lpad value, but still gets a non-zero lpad value, e.g.

That's too aggressive, so let me leave this along here, I would suggest implement that as a linker extension and should enable by an option.

Copy link

@mylai-mtk mylai-mtk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your updates. I've found a few wording problems, but I think the spec is in good shape (at least for now 😂).

@@ -548,7 +548,9 @@ Description:: Additional information about the relocation
<| S - P
.2+| 65 .2+| TLSDESC_CALL .2+| Static | .2+| Annotate call to TLS descriptor resolver function, `%tlsdesc_call(address of %tlsdesc_hi)`, for relaxation purposes only
<|
.2+| 66-190 .2+| *Reserved* .2+| - | .2+| Reserved for future standard use
.2+| 66 .2+| LPAD .2+| Static | .2+| Annotates the landing pad instruction inserted at the beginning of the function.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the relocation need not be restricted to annotate lpads as function beginnings. Lpads in the middle of functions can also be annotated by this relocation, given that the relocation's symbol field maybe empty.

Comment on lines +1585 to +1586
to generate PLT with landing pads. The static linker is required to use the
landing pad values provided by this section when generating lpad instructions

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The static linker is required to use the landing pad values provided by this section when generating lpad instructions

The information contained in .riscv.lpadinfo is not limited to generating lpad insns. It's also used for generating lui t2, <lpad_value> in PLT, so the better wording is "The static linker is required to use the landing pad values provided by this section when generating PLT entries ..."

} Elf64_Lpadinfo;
```

The `lpi_name` field is the index into the string table for the symbol name,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The lpi_name field is the index into the string table for the symbol name

You forget to update this sentence. My updated wording would be:

The lpi_sym field refers to a symbol by indexing into in the symbol table. The indexed symbol table is .dynsym for shared libraries, and .symtab for other ELF files.

@@ -1612,6 +1693,12 @@ is not enough for the disassembler to disassemble the `rv64gcv` version
correctly. Specifying ISA string appropriately with the two memcpy instruction
mapping symbols helps the disassembler to disassemble instructions correctly.

The mapping symbol for the landing pad (`$s`) provides additional information
for the labeled landing pad scheme. This mapping symbol may be generated when
setting up a landing pad value (e.g., `auipc t2, %lpad_hash("FvvE")` will

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you mean lui t2, %lpad_hash(...)?

BTW, the condition under which this mapping symbol can be emitted could actually be summarized as "if there's a %lpad_hash(...), an lpad mapping symbol is emitted". Would you mind to restructure the spec around the summarized condition so that it's more versatile compared to the current spec that explicitly constrains the emission condition of the symbol to the 2 listed "intentions"?

@@ -1612,6 +1693,12 @@ is not enough for the disassembler to disassemble the `rv64gcv` version
correctly. Specifying ISA string appropriately with the two memcpy instruction
mapping symbols helps the disassembler to disassemble instructions correctly.

The mapping symbol for the landing pad (`$s`) provides additional information
Copy link

@mylai-mtk mylai-mtk Jan 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The optional ISA information, when present, will be used until the next instruction mapping symbol.

This existing spec for the interpretation of mapping symbols conflicts with the lpad mapping symbol.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants