[gcc(refs/vendors/ARM/heads/morello)] Fix bugs around automatic capability alignment

[gcc(refs/vendors/ARM/heads/morello)] Fix bugs around automatic capability alignment

[Matthew Malcomson]

https://gcc.gnu.org/g:4b7272b8c06eed8d3667412c063e61b8c5b9f292

commit 4b7272b8c06eed8d3667412c063e61b8c5b9f292
Author: Matthew Malcomson <matthew.malcomson@arm.com>
Date:   Thu Feb 3 17:37:16 2022 +0000

    Fix bugs around automatic capability alignment
    
    In commit a77f6664310 we always aligned things that were being written
    out as capabilities.  This triggered some failures in the testsuite,
    which pointed out that the idea was mistaken.
    
    The idea was based on the fact that loading an unaligned capability
    triggers a fault hence thinking that we would only ever want to write
    out aligned capabilities.  The choice to allow unaligned capabilities
    being written out had already been made the other way, and that makes
    more sense.  Allowing unaligned capabilities means that packed
    structures with pointers in them are allowed (while we have a warning
    on the unaligned capability).
    
    This patch reverts that part of the commit, and also ensures we never use
    `capinit` or `chericap` to emit capabilities that we want unaligned.
    
    This adjustment is necessary since the assembler adds alignment when it sees
    either of these directives.  Hence when trying to emit an unaligned capability
    we need to emit it without these directives.
    
    That change to emit unaligned capabilities with a different directive uncovered
    an existing bug from the "precisely bounded capabilities" implementation (see
    2167d4d7c41).  A few places mistakenly passed the alignment to the new hook in
    bits, while the hook takes alignment in bytes, while others used the return
    value from the new hook as if it were bits when it was in fact bytes.
    
    We do not change the definition of the hook, since the units of the
    corresponding hook are also bytes and we feel that matching these units still
    seems like the least error-prone approach.
    
    Here we simply use the helper function `alignment_pad_from_bits` that uses bits
    for this alignment argument.
    
    This patch does not come with a testsuite change, it is tested using the
    existing testcases which emit unaligned capabilities and/or those which emit
    aligned capabiilies going through those codepaths which have been modified.
    (tests in the testsuite which have been seen to be affected by broken
    implementations of this patch are:
     - gcc.c-torture/execute/20010915-1.c
     - gcc.c-torture/execute/920501-5.c
     - gcc.c-torture/execute/980223.c
     - gcc.c-torture/execute/builtins/strpbrk.c
     - gcc.dg/20000906-1.c
     - gcc.dg/torture/pr52530.c
     - gcc.dg/torture/pr53908.c
     - gcc.target/aarch64/aapcs64/morello-test_8.c
     - gcc.target/aarch64/morello/label-addressing-includes-lsb.c)
    
    For ease of review, a description of the changes:
    - assemble_noswitch_variable takes its alignment in bits hence we should pass
      the alignment argument of targetm.data_padding_size correspondingly adjusted.
      (can see this alignment is given in bits by the rest of the function).
    - assemble_constant_contents mentions that it takes alignment in bits in the
      comment above the function.
    - output_constant_def_contents passes its alignment to
      assemble_constant_contents hence its internal align variable should stay
      represented as bits.
    - output_constant_pool_1 passes its `align_used` internal variable to
      `assemble_align` hence that should be in bits.
    - place_block_symbol already did its calculations in bits, but we have a
      helper function which handles the case when there was a bitwise alignment and
      targetm.data_alignment did not change the alignment.  Hence we change the
      code to use that helper function.

Diff:
---
 gcc/config/aarch64/aarch64.c | 22 ++++++++++++++--------
 gcc/varasm.c                 | 35 +++++++++++++++++------------------
 2 files changed, 31 insertions(+), 26 deletions(-)

diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 1a684a2b66f..52d86e47924 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -20256,16 +20256,12 @@ cap_offset_known_outside_bounds (rtx label, rtx maybe_constant, tree *declp,
 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)
 	{
-	  fputs ("\t.p2align\t4\n", asm_out_file);
+	  if (aligned_p)
+	    fputs ("\t.p2align\t4\n", asm_out_file);
 	  fputs ("\t.zero\t16\n", asm_out_file);
 	  return true;
 	}
@@ -20306,7 +20302,8 @@ aarch64_asm_output_capability (rtx x, unsigned int size, int aligned_p)
       && CONST_NULL_P (XEXP (x, 0))
       && size == 16)
     {
-      fputs ("\t.p2align\t4\n", asm_out_file);
+      if (aligned_p)
+	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);
     }
@@ -20334,9 +20331,18 @@ aarch64_asm_output_capability (rtx x, unsigned int size, int aligned_p)
 	 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);
+      if (aligned_p)
+	fputs ("\t.p2align\t4\n", asm_out_file);
       ret = targetm.asm_out.integer (drop_capability (x), offset_size, aligned_p);
     }
+  else if (! aligned_p)
+    {
+      /* If we want to emit a capability but it is not aligned, we can't use
+	 the `capinit` or `chericap` directives since they include alignment to
+	 a capability boundary.  */
+      ret = targetm.asm_out.integer (drop_capability (x), offset_size,
+				     aligned_p);
+    }
   else
     {
       fputs ("\t.capinit\t", asm_out_file);
diff --git a/gcc/varasm.c b/gcc/varasm.c
index c42519a316f..e36d9197c9a 100644
--- a/gcc/varasm.c
+++ b/gcc/varasm.c
@@ -2098,7 +2098,7 @@ assemble_noswitch_variable (tree decl, const char *name, section *sect,
   unsigned HOST_WIDE_INT size, rounded;
 
   size = tree_to_uhwi (DECL_SIZE_UNIT (decl));
-  size += targetm.data_padding_size (size, align, decl);
+  size += targetm.data_padding_size (size, align / BITS_PER_UNIT, decl);
   rounded = size;
 
   if ((flag_sanitize & SANITIZE_ADDRESS) && asan_protect_global (decl))
@@ -3620,7 +3620,7 @@ assemble_constant_contents (tree exp, const char *label, unsigned int align,
   HOST_WIDE_INT size;
 
   size = get_constant_size (exp);
-  size += targetm.data_padding_size (size, align, decl);
+  size += targetm.data_padding_size (size, align / BITS_PER_UNIT, decl);
 
   /* Do any machine/system dependent processing of the constant.  */
   targetm.asm_out.declare_constant_name (asm_out_file, label, exp, size);
@@ -3669,11 +3669,9 @@ output_constant_def_contents (rtx symbol)
 		   : symtab_node::get (decl)->definition_alignment ());
       section *sect = get_constant_section (exp, align);
       switch_to_section (sect);
-      align = targetm.data_alignment (get_constant_size (exp),
-				      align / BITS_PER_UNIT,
-				      decl);
+      align = alignment_pad_from_bits (get_constant_size (exp), align, decl);
       if (align)
-	ASM_OUTPUT_ALIGN (asm_out_file, floor_log2 (align));
+	ASM_OUTPUT_ALIGN (asm_out_file, floor_log2 (align/BITS_PER_UNIT));
       assemble_constant_contents (exp, XSTR (symbol, 0), align,
 				  (sect->common.flags & SECTION_MERGE)
 				  && (sect->common.flags & SECTION_STRINGS),
@@ -4169,8 +4167,10 @@ output_constant_pool_1 (class constant_descriptor_rtx *desc,
     }
 
   uint64_t size = GET_MODE_SIZE (desc->mode);
-  uint64_t align_used = targetm.data_alignment (size, align, NULL_TREE);
-  uint64_t padding = targetm.data_padding_size (size, align_used, NULL_TREE);
+  uint64_t align_used = alignment_pad_from_bits (size, align, NULL_TREE);
+  uint64_t padding = targetm.data_padding_size (size,
+						align_used / BITS_PER_UNIT,
+						NULL_TREE);
 
 #ifdef ASM_OUTPUT_SPECIAL_POOL_ENTRY
   ASM_OUTPUT_SPECIAL_POOL_ENTRY (asm_out_file, x, desc->mode,
@@ -7746,10 +7746,9 @@ place_block_symbol (rtx symbol)
       desc = SYMBOL_REF_CONSTANT (symbol);
       alignment = desc->align;
       size = GET_MODE_SIZE (desc->mode);
-      alignment = targetm.data_alignment (size, alignment / BITS_PER_UNIT,
-					  NULL_TREE);
-      size += targetm.data_padding_size (size, alignment, NULL_TREE);
-      alignment *= BITS_PER_UNIT;
+      alignment = alignment_pad_from_bits (size, alignment, NULL_TREE);
+      size += targetm.data_padding_size (size, alignment / BITS_PER_UNIT,
+					 NULL_TREE);
     }
   else if (TREE_CONSTANT_POOL_ADDRESS_P (symbol))
     {
@@ -7757,9 +7756,9 @@ place_block_symbol (rtx symbol)
       gcc_checking_assert (DECL_IN_CONSTANT_POOL (decl));
       alignment = DECL_ALIGN (decl);
       size = get_constant_size (DECL_INITIAL (decl));
-      alignment = targetm.data_alignment (size, alignment / BITS_PER_UNIT, decl);
-      size += targetm.data_padding_size (size, alignment, decl);
-      alignment *= BITS_PER_UNIT;
+      alignment = alignment_pad_from_bits (size, alignment, decl);
+      size += targetm.data_padding_size (size, alignment / BITS_PER_UNIT,
+					 decl);
       if ((flag_sanitize & SANITIZE_ADDRESS)
 	  && TREE_CODE (DECL_INITIAL (decl)) == STRING_CST
 	  && asan_protect_global (DECL_INITIAL (decl)))
@@ -7789,9 +7788,9 @@ place_block_symbol (rtx symbol)
 	}
       alignment = get_variable_align (decl);
       size = tree_to_uhwi (DECL_SIZE_UNIT (decl));
-      alignment = targetm.data_alignment (size, alignment / BITS_PER_UNIT, decl);
-      size += targetm.data_padding_size (size, alignment, decl);
-      alignment *= BITS_PER_UNIT;
+      alignment = alignment_pad_from_bits (size, alignment, decl);
+      size += targetm.data_padding_size (size, alignment / BITS_PER_UNIT,
+					 decl);
       if ((flag_sanitize & SANITIZE_ADDRESS)
 	  && asan_protect_global (decl))
 	{