It looks like the only value that makes sense is Error - any other policy (existing or not) would potentially lead to meaningfully different code generated with or without LTO.

Thank you for your comments. I'd like to have the same behaviour with or without LTO, which is apparently not the case with this patch.

Ping.

Ping

Ping,

LGTM.

Ping.

LGTM. It'd be nice if we could get someone non-Arm to have a look too. though.

LGTM, as soon as D85649 is accepted (so they stay in sync).

Emit function-level LLVM IR attributes only when there's a GCC-style function attribute and be explicit with enabling/disabling.
Fixed an error where "none" in deferred to module attributes, instead of overriding them.

No, not yet.

In addition to the disabling of BTI in D81251, there's an issue that we explicitly disable BTI via branch-protection=none, the attribute would just be missing and we'll pick up the module attributes, which is not what we want.

Ping?

In D85649#2231236, @tellenbach wrote:

Does this conflict with D80791?

It does, we're using different module attributes.

In D85649 I suggested a different version of module flags, which is a bit nicer to use, e.g. one can say just

getModuleFlag("sign-return-address-with-bkey") != nullptr

instead of a) checking for the flag presence, b) getting its value and c) comparing it to a set of strings, which is
way too verbose.

Thanks!

Ping?

Ping?

In D85649#2207922, @nickdesaulniers wrote:

Is there a test for no module attributes + function level attribute enabling pac/bti that tests that just that function gets the proper handling?

In D80791#2210124, @danielkiss wrote:

it is not useful to have a bti annotated function unless everything else is bti compatible too: it is all or nothing per elf module.

This is false. Some functions in an elf module could be in a guarded region, some in a non-guarded region. Some function may always
be called in a "BTI-safe" way, which may be unknown to the compiler.

Right now the elf and all of the text sections considered BTI enabled or not. The dynamic linkers/loaders can't support this
use case without additional information to be encoded somewhere (and specified). To support such we need to consider grouping/align to page
boundaries these functions in the linker because BTI could be controlled by flags in PTE.
With the current spec this usecase is not supported in this way. The user have to link the BTI protected code into another elf.
Side note: The force-bti linker option can't work with half BTI enabled objects.

In D80791#2209624, @nsz wrote:

In D80791#2207203, @chill wrote:

I would prefer to avoid the situation where the markings of two otherwise identical files were different,
depending on how the files were produced, no matter if it was a common or a special case.

i don't see why it is desirable to silently get marking on an object file if function definitions happen to be bti compatible in it:

Well, it wasn't completely broken, just a little bit :D

Updated the patch to unconditionally include PAC stripping instructions, NOP-space ones to non-NON-space ones, depending on
whether v8.3-a (soo to be '+pa') feature.

I would prefer to avoid the situation where the markings of two otherwise identical files were different,
depending on how the files were produced, no matter if it was a common or a special case.

In D80791#2206853, @nsz wrote:

i think that cannot work.

the implementation is free to inject arbitrary code into
user code so if the user does not tell the implementation
that it wants the entire tu to be bti safe then non-bti
code can end up in there. (e.g. ctor of an instrumentation
that is not realated to any particular function with the
bti marking)

In D80791#2196598, @nsz wrote:

the assumption is that the intended branch protection is implied via cmdline flags for the tu and function attributes are only used in source code for some hack.

I don't share this assumption. I find it just as valid to control the PAC/BTI with things like:

#ifdef ENABLE_BTI
#define BTI_FUNC __attribute__((target("branch-protection=bti")))
#else
#define BTI_FUNC

In D75181#2193447, @danielkiss wrote:

I don't see any other alternative option, I'm open to any other idea.

My original idea was to pass options to LLVM. I'll come up with a patch in a day or two (if it works) and then we'll see.

I'm in favour of changing these patches to unconditionally emit XPACLRI (i.e. HINT #whatever).

This approach looks way too hackish to me with multiple opposing attributes ("sign-return-address" vs. "ignore-sign-return-address")
and some convoluted logic to resolve the contradiction.

In D80791#2164543, @danielkiss wrote:

If any function has the attribute "sign-return-address", then the output note
section should have PAC bit set. The return address signing is completely local
to the function, and functions with or without return address signing can be
freely mixed with each other.

That is true PAC and non-PAC functions can be mixed.
Does one function makes the "all executable sections" pac-ret enabled?

Is this patch needed anymore?

In D75044#2186973, @chill wrote:

Let's postpone this just for a little bit, to settle on an approach to depth > 0.

Let's postpone this just for a little bit, to settle on an approach to depth > 0.

LGTM, thanks!

LGTM. Thanks!

In D75044#2165496, @chill wrote:

The issue is that the definition of the instructions XPAC{D,I} is incorrect: it does not mention at all the operand to those insns.

Err, they do mention the operand, but only as an input one, it should be input/output.

I'm afraid the patch does not work yet. For example, when the following program

I don't think this behaviour is correct with regard to the specification (AAELF64 2020Q2):

Needs a regression test. This patch and the dependent patch clash, better with a single patch.

Thanks!

PIng.

Yeah, I'd prefer that, but then I'm hesitant to change APIs.

Pretty straightforward, LGTM. I'd suggest rewording the title (presumably commit message summary) into something like "Do not coerce bfloat arguments and returns to integers", as we're obviously still lowering C and C++ to LLVM LR.§§

Updated to take into account liveness of registers. Whenever we blanket push registers, the live ones are ordinary operands,
the dead ones are Undef operands.

LGTM, thank you.

I see.

I'm sorry, I don't understand the issue. Certainly it's the compiler (driver) responsibility to setup include paths according to the selected target.
How do you trigger a problem?

The last version of the patch broke compilation with expensive checks - the machine verifier
reported a score of lifetime errors.
In this update, we are more precise with the RegState flags when creating instructions
to save/restore/clear registers around non-secure calls and before return to non-secure state.