[gcc(refs/vendors/ARM/heads/morello)] Do not write known-bad capabilities out
Matthew Malcomson
matmal01@gcc.gnu.org
Mon Feb 28 12:08:30 GMT 2022
https://gcc.gnu.org/g:6404652ab0fc9904b48224d4a8a2e955f30c4e5e
commit 6404652ab0fc9904b48224d4a8a2e955f30c4e5e
Author: Matthew Malcomson <matthew.malcomson@arm.com>
Date: Tue Jan 18 10:36:30 2022 +0000
Do not write known-bad capabilities out
This patch contains two parts. The first is that when writing a
`capinit` or `chericap` relocation on a symbol plus addend where the
addend is known to be outside of the symbols range we do not use the
relevant directive but instead output an address with NULL metadata.
This change was made because the linker does not accept such `capinit`
directives -- it errors out.
We choose to warn yet still emit a bad capability rather than error out
since there is the chance this bad capability will never be used
depending on some other part of the code. If a codebase would benefit
from erroring out then it can use -Werror. We do not silently accept
such a feature since we judGE this would not usually be something
expected.
In order to account for this we will want to turn the error in GNU ld
into a warning.
For comparison, LLVM does not warn or error out on such invalid
capabilities. The particular added in this patch emits a .capinit
directive for the base of the string and adds the invalid offset in
function code, but linking an object with LLD that has the CAPINIT
relocation with an offset past the bounds of the object it refers to is
still silently accepted.
This patch also includes changes to ensure alignment of anything that is
output as if it were a capability even if in reality the binary will not
end up with a capability.
This is needed since an `ldr` with a capability register requires a
16bit aligned address and will crash otherwise. Usually the assembler
ensures alignment when it sees a `capinit` or `chericap` directive but
this obviously doesn't happen when that is not what we emit.
We care about this case since there is the possibility that a user will
generate an invalid capability and load it but be happy with an invalid
capability -- this could be because they do a test before using it or
are still happy with an invalid capability that has the address of
something.
In order to put this warning under the `-Wcheri-provenance` option we
made that `-Wcheri-provenance` option something accessible throughout
the compiler.
This change triggers a few new failures in the testsuite, these are all
because of the extra warnings acting as they should and hence triggering
new error messages that the testsuite did not expect.
The new failures are below, we fix up these by adding the appropriate
extra dg-warning directives.
gcc.dg/format/plus-1.c
gcc.dg/format/plus-1.c
gcc.dg/pr24683.c
gcc.dg/pr47727.c
gcc.dg/Warray-bounds-29.c
gcc.dg/Warray-bounds-31.c
gcc.dg/Warray-bounds-58.c
A note on implementation:
We put this in the AArch64 backend because we do not know what other
targets may or may not want to do in such situations. Requesting an
out-of-bounds capability would be a valid choice.
Essentially the decision is being left for later when we have more
information about cases where this can come up. All cases in the
testsuite so far do not appear to be cases where the valaue would be
expected to be dereferenced validly at runtime.
Diff:
---
gcc/c-family/c.opt | 4 --
gcc/common.opt | 5 ++
gcc/config/aarch64/aarch64.c | 80 +++++++++++++++++++---
gcc/testsuite/gcc.dg/Warray-bounds-29.c | 2 +-
gcc/testsuite/gcc.dg/Warray-bounds-31.c | 2 +-
gcc/testsuite/gcc.dg/Warray-bounds-58.c | 2 +-
gcc/testsuite/gcc.dg/format/plus-1.c | 2 +-
gcc/testsuite/gcc.dg/pr24683.c | 2 +-
gcc/testsuite/gcc.dg/pr47727.c | 2 +-
.../aarch64/morello/known-outside-bounds.c | 8 +++
10 files changed, 89 insertions(+), 20 deletions(-)
diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 00c285dde93..2b1aca16eb4 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -707,10 +707,6 @@ Wint-to-pointer-cast
C ObjC C++ ObjC++ Var(warn_int_to_pointer_cast) Init(1) Warning
Warn when there is a cast to a pointer from an integer of a different size.
-Wcheri-provenance
-C C++ Var(warn_cheri_provenance) Init(1) Warning
-Warn when an expression has ambiguous pointer provenance.
-
Winvalid-offsetof
C++ ObjC++ Var(warn_invalid_offsetof) Init(1) Warning
Warn about invalid uses of the \"offsetof\" macro.
diff --git a/gcc/common.opt b/gcc/common.opt
index 1b22bc59fcb..a95f0ffec5a 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -574,6 +574,11 @@ Wcheri-bounds
Common Var(warn_cheri_bounds) Init(1) Warning
Warn when an object can not be aligned to ensure non-overlapping bounds.
+Wcheri-provenance
+Common Init(1) Warning
+Warn when an expression has ambiguous pointer provenance or will be known
+invalid.
+
Wattribute-warning
Common Var(warn_attribute_warning) Init(1) Warning
Warn about uses of __attribute__((warning)) declarations.
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 570f634af1f..1a684a2b66f 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -20223,17 +20223,49 @@ aarch64_print_patchable_function_entry (FILE *file,
default_print_patchable_function_entry (file, patch_area_size, record_p);
}
+static bool
+cap_offset_known_outside_bounds (rtx label, rtx maybe_constant, tree *declp,
+ unsigned HOST_WIDE_INT *offp)
+{
+ if (!CONST_INT_P (maybe_constant))
+ return false;
+ HOST_WIDE_INT off = INTVAL (maybe_constant);
+ machine_mode mode = GET_MODE (label);
+
+ if (POINTER_PLUS_P (label))
+ return cap_offset_known_outside_bounds
+ (XEXP (label, 0),
+ plus_constant (mode, XEXP (label, 1), off),
+ declp, offp);
+
+ if (!SYMBOL_REF_P (label) || !SYMBOL_REF_DECL (label))
+ return false;
+ tree decl = SYMBOL_REF_DECL (label);
+ if (DECL_SIZE_UNIT (decl)
+ && (off < 0
+ || (unsigned HOST_WIDE_INT)off > tree_to_uhwi (DECL_SIZE_UNIT (decl))))
+ {
+ *declp = decl;
+ *offp = off;
+ return true;
+ }
+ return false;
+}
+
/* Implement ASM_OUTPUT_CAPABILITY. */
bool
aarch64_asm_output_capability (rtx x, unsigned int size, int aligned_p)
{
+ /* Capability loads must be aligned on Morello, so we must emit an aligned
+ capability. This is automatically done by the assembler if we're using
+ `capinit` or `chericap`, but when we emit an integer we need to manually
+ do that. */
+ aligned_p = aligned_p || !TARGET_CAPABILITY_FAKE;
if (CONST_NULL_P (x))
{
if (size == 16)
{
- if (aligned_p)
- fputs ("\t.p2align\t4\n", asm_out_file);
-
+ fputs ("\t.p2align\t4\n", asm_out_file);
fputs ("\t.zero\t16\n", asm_out_file);
return true;
}
@@ -20274,17 +20306,45 @@ aarch64_asm_output_capability (rtx x, unsigned int size, int aligned_p)
&& CONST_NULL_P (XEXP (x, 0))
&& size == 16)
{
- if (aligned_p)
- fputs ("\t.p2align\t4\n", asm_out_file);
-
+ fputs ("\t.p2align\t4\n", asm_out_file);
ret = targetm.asm_out.integer (XEXP (x, 1), 8, 0);
return ret && targetm.asm_out.integer (const0_rtx, 8, 0);
}
- fputs ("\t.capinit\t", asm_out_file);
- output_addr_const (asm_out_file, x);
- fputc ('\n', asm_out_file);
- ret = targetm.asm_out.integer (const0_rtx, offset_size, aligned_p);
+ tree warning_decl;
+ unsigned HOST_WIDE_INT warning_offset;
+ if (GET_CODE (x) == POINTER_PLUS
+ && cap_offset_known_outside_bounds (XEXP (x, 0), XEXP (x, 1),
+ &warning_decl, &warning_offset))
+ {
+ /* MORELLO TODO Would be nice to improve this diagnostic somehow.
+ One problem is that we can tell we have an issue here but we don't
+ have the information about what these things represent to the user
+ hence we give something that is a little unclear.
+ Another problem is that we repeat the same warning if the rest of the
+ compiler has decided to output the same constant plus offset in
+ different places. This doesn't seem like too bad to worry about right
+ now. */
+ if (warning (OPT_Wcheri_provenance,
+ "%qld offset is outside of %qD capability from which it is offset",
+ warning_offset, warning_decl))
+ inform (UNKNOWN_LOCATION,
+ "-O1 -Warray-bounds may provide a more precise diagnostic");
+ /* If we know the offset is outside of the bounds of this label, avoid
+ requesting a capability for it. This would cause a linker error since
+ the linker refuses to generate such invalid-by-definition
+ capabilities. */
+ fputs ("\t.p2align\t4\n", asm_out_file);
+ ret = targetm.asm_out.integer (drop_capability (x), offset_size, aligned_p);
+ }
+ else
+ {
+ fputs ("\t.capinit\t", asm_out_file);
+ output_addr_const (asm_out_file, x);
+ fputc ('\n', asm_out_file);
+ ret = targetm.asm_out.integer (const0_rtx, offset_size, aligned_p);
+ }
+
return ret && targetm.asm_out.integer (const0_rtx, offset_size, aligned_p);
}
diff --git a/gcc/testsuite/gcc.dg/Warray-bounds-29.c b/gcc/testsuite/gcc.dg/Warray-bounds-29.c
index 72c5d1cecf8..3862241922c 100644
--- a/gcc/testsuite/gcc.dg/Warray-bounds-29.c
+++ b/gcc/testsuite/gcc.dg/Warray-bounds-29.c
@@ -147,4 +147,4 @@ void test_wide (void)
T (p4[-2]);
T (p4[-1]);
T (p4[ 0]); /* { dg-warning "array subscript \\\[4, 8] is outside array bounds of .\[a-z \]+\\\[4]." } */
-}
+} /* { dg-warning "offset is outside of .* capability from which it is offset" "" { target *-*-* } } */
diff --git a/gcc/testsuite/gcc.dg/Warray-bounds-31.c b/gcc/testsuite/gcc.dg/Warray-bounds-31.c
index 389afaf045d..785a231cf91 100644
--- a/gcc/testsuite/gcc.dg/Warray-bounds-31.c
+++ b/gcc/testsuite/gcc.dg/Warray-bounds-31.c
@@ -245,4 +245,4 @@ void narrow_ptr_index_range (void)
p = S7 + SR (4, 6);
T (p[5]); /* { dg-warning "array subscript \\\[9, 11] is outside array bounds of .char\\\[8]." } */
-}
+} /* { dg-warning "offset is outside of .* capability from which it is offset" "" { target *-*-* } } */
diff --git a/gcc/testsuite/gcc.dg/Warray-bounds-58.c b/gcc/testsuite/gcc.dg/Warray-bounds-58.c
index 7bd6df2bf2e..9e74d6ab020 100644
--- a/gcc/testsuite/gcc.dg/Warray-bounds-58.c
+++ b/gcc/testsuite/gcc.dg/Warray-bounds-58.c
@@ -78,4 +78,4 @@ void fax_static (void)
sink (strlen (ax3.a + 3));
sink (strlen (ax3.a + 4)); // { dg-warning "\\\[-Warray-bounds" "pr93514" { xfail *-*-* } }
sink (strlen (ax3.a + 5)); // { dg-warning "\\\[-Warray-bounds" }
-}
+} /* { dg-warning "offset is outside of .* capability from which it is offset" "" { target *-*-* } } */
diff --git a/gcc/testsuite/gcc.dg/format/plus-1.c b/gcc/testsuite/gcc.dg/format/plus-1.c
index 02a213d417d..692554a1633 100644
--- a/gcc/testsuite/gcc.dg/format/plus-1.c
+++ b/gcc/testsuite/gcc.dg/format/plus-1.c
@@ -17,4 +17,4 @@ foo (int i)
printf ("%d\n" + 10); /* { dg-warning "not a string" "too large addend" } */
printf ("%d\n" - 1, i); /* { dg-warning "not a string" "minus constant" } */
printf ("%d\n" + -1, i); /* { dg-warning "not a string" "negative addend" } */
-}
+} /* { dg-warning "offset is outside of .* capability from which it is offset" "" { target *-*-* } } */
diff --git a/gcc/testsuite/gcc.dg/pr24683.c b/gcc/testsuite/gcc.dg/pr24683.c
index 23cb3ab4b01..890a6e28b7b 100644
--- a/gcc/testsuite/gcc.dg/pr24683.c
+++ b/gcc/testsuite/gcc.dg/pr24683.c
@@ -8,4 +8,4 @@ void final(unsigned int j)
for (i = 0; i < 8; i++)
for (; j + 63 < 1; j += 64)
block = (const int *) &data[j];
-}
+} /* { dg-warning "offset is outside of .* capability from which it is offset" "" { target *-*-* } } */
diff --git a/gcc/testsuite/gcc.dg/pr47727.c b/gcc/testsuite/gcc.dg/pr47727.c
index 1ce7c360117..6fdcab8a147 100644
--- a/gcc/testsuite/gcc.dg/pr47727.c
+++ b/gcc/testsuite/gcc.dg/pr47727.c
@@ -9,4 +9,4 @@ __do_global_ctors_aux (void)
func_ptr *p;
for (p = __CTOR_END__ - 1; *p != (func_ptr) -1; p--)
(*p) ();
-}
+} /* { dg-warning "offset is outside of .* capability from which it is offset" "" { target *-*-* } } */
diff --git a/gcc/testsuite/gcc.target/aarch64/morello/known-outside-bounds.c b/gcc/testsuite/gcc.target/aarch64/morello/known-outside-bounds.c
new file mode 100644
index 00000000000..896861c2fd6
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/morello/known-outside-bounds.c
@@ -0,0 +1,8 @@
+/* { dg-do compile { target aarch64*-*-* } } */
+static const char *const myvar = "" + 10;
+const char *
+foo ()
+{
+ return myvar;
+} /* { dg-warning "offset is outside of .* capability from which it is offset" "" { target *-*-* } } */
+
More information about the Gcc-cvs
mailing list