[2/2] asan: Align .LASANPC on function boundary

Message ID 20231207121005.3425208-3-iii@linux.ibm.com
State New
Headers
Series asan: Align .LASANPC on function boundary |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gcc_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gcc_check--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gcc_check--master-aarch64 success Testing passed

Commit Message

Ilya Leoshkevich Dec. 7, 2023, 12:08 p.m. UTC
  GCC can emit code between the function label and the .LASANPC label,
making the latter unaligned.  Some architectures cannot load unaligned
labels directly and require literal pool entries, which is inefficient.

Move the invocation of asan_function_start to
ASM_OUTPUT_FUNCTION_LABEL, which guarantees that no additional code is
emitted.  This allows setting the .LASANPC label alignment to the
respective function alignment.
---
 gcc/asan.cc             |  6 ++----
 gcc/config/i386/i386.cc |  2 +-
 gcc/config/s390/s390.cc |  2 +-
 gcc/defaults.h          |  2 +-
 gcc/final.cc            |  3 ---
 gcc/output.h            |  4 ++++
 gcc/varasm.cc           | 10 ++++++++++
 7 files changed, 19 insertions(+), 10 deletions(-)
  

Comments

Jakub Jelinek Dec. 7, 2023, 12:25 p.m. UTC | #1
On Thu, Dec 07, 2023 at 01:08:27PM +0100, Ilya Leoshkevich wrote:
> GCC can emit code between the function label and the .LASANPC label,
> making the latter unaligned.  Some architectures cannot load unaligned
> labels directly and require literal pool entries, which is inefficient.
> 
> Move the invocation of asan_function_start to
> ASM_OUTPUT_FUNCTION_LABEL, which guarantees that no additional code is
> emitted.  This allows setting the .LASANPC label alignment to the
> respective function alignment.

> --- a/gcc/output.h
> +++ b/gcc/output.h
> @@ -178,6 +178,10 @@ extern void assemble_asm (tree);
>  /* Get the function's name from a decl, as described by its RTL.  */
>  extern const char *get_fnname_from_decl (tree);
>  
> +/* Output function label, possibly with accompanying metadata.  No additional
> +   code or data is output after the label.  */

Please copy the above comment also

> --- a/gcc/varasm.cc
> +++ b/gcc/varasm.cc
> @@ -61,6 +61,7 @@ along with GCC; see the file COPYING3.  If not see
>  #include "alloc-pool.h"
>  #include "toplev.h"
>  #include "opts.h"
> +#include "asan.h"
>  
>  /* The (assembler) name of the first globally-visible object output.  */
>  extern GTY(()) const char *first_global_object_name;
> @@ -1835,6 +1836,15 @@ get_fnname_from_decl (tree decl)
>    return XSTR (x, 0);
>  }
>  

here.

> +void assemble_function_label_raw (FILE *file, const char *name)

Also, newline rather than space after void.

> +{
> +  ASM_OUTPUT_LABEL (file, name);
> +  if ((flag_sanitize & SANITIZE_ADDRESS)
> +      /* Notify ASAN only about the first function label.  */
> +      && (in_cold_section_p == first_function_block_is_cold))
> +    asan_function_start ();
> +}

With this the asan.cc/varasm.cc changes look good to me, for the rest I'd
prefer to hear what Jeff says.

	Jakub
  

Patch

diff --git a/gcc/asan.cc b/gcc/asan.cc
index 8d0ffb497cc..48738244aba 100644
--- a/gcc/asan.cc
+++ b/gcc/asan.cc
@@ -1481,10 +1481,7 @@  asan_clear_shadow (rtx shadow_mem, HOST_WIDE_INT len)
 void
 asan_function_start (void)
 {
-  section *fnsec = function_section (current_function_decl);
-  switch_to_section (fnsec);
-  ASM_OUTPUT_DEBUG_LABEL (asm_out_file, "LASANPC",
-			 current_function_funcdef_no);
+  ASM_OUTPUT_DEBUG_LABEL (asm_out_file, "LASANPC", current_function_funcdef_no);
 }
 
 /* Return number of shadow bytes that are occupied by a local variable
@@ -2006,6 +2003,7 @@  asan_emit_stack_protection (rtx base, rtx pbase, unsigned int alignb,
   DECL_INITIAL (decl) = decl;
   TREE_ASM_WRITTEN (decl) = 1;
   TREE_ASM_WRITTEN (id) = 1;
+  DECL_ALIGN_RAW (decl) = DECL_ALIGN_RAW (current_function_decl);
   emit_move_insn (mem, expand_normal (build_fold_addr_expr (decl)));
   shadow_base = expand_binop (Pmode, lshr_optab, base,
 			      gen_int_shift_amount (Pmode, ASAN_SHADOW_SHIFT),
diff --git a/gcc/config/i386/i386.cc b/gcc/config/i386/i386.cc
index 7c5cab4e2c6..a552a300b69 100644
--- a/gcc/config/i386/i386.cc
+++ b/gcc/config/i386/i386.cc
@@ -1640,7 +1640,7 @@  ix86_asm_output_function_label (FILE *out_file, const char *fname,
   SUBTARGET_ASM_UNWIND_INIT (out_file);
 #endif
 
-  ASM_OUTPUT_LABEL (out_file, fname);
+  assemble_function_label_raw (out_file, fname);
 
   /* Output magic byte marker, if hot-patch attribute is set.  */
   if (is_ms_hook)
diff --git a/gcc/config/s390/s390.cc b/gcc/config/s390/s390.cc
index 044de874590..a022db230db 100644
--- a/gcc/config/s390/s390.cc
+++ b/gcc/config/s390/s390.cc
@@ -8323,7 +8323,7 @@  s390_asm_output_function_label (FILE *out_file, const char *fname,
       asm_fprintf (out_file, "\t# fn:%s wd%d\n", fname,
 		   s390_warn_dynamicstack_p);
     }
-  ASM_OUTPUT_LABEL (out_file, fname);
+  assemble_function_label_raw (out_file, fname);
   if (hw_after > 0)
     asm_fprintf (out_file,
 		 "\t# post-label NOPs for hotpatch (%d halfwords)\n",
diff --git a/gcc/defaults.h b/gcc/defaults.h
index dc6f09cacae..153d3cd32c0 100644
--- a/gcc/defaults.h
+++ b/gcc/defaults.h
@@ -150,7 +150,7 @@  see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
 
 #ifndef ASM_OUTPUT_FUNCTION_LABEL
 #define ASM_OUTPUT_FUNCTION_LABEL(FILE, NAME, DECL) \
-  ASM_OUTPUT_LABEL ((FILE), (NAME))
+  assemble_function_label_raw ((FILE), (NAME))
 #endif
 
 /* Output the definition of a compiler-generated label named NAME.  */
diff --git a/gcc/final.cc b/gcc/final.cc
index e6f1b1e166b..5e21aedf8ed 100644
--- a/gcc/final.cc
+++ b/gcc/final.cc
@@ -1686,9 +1686,6 @@  final_start_function_1 (rtx_insn **firstp, FILE *file, int *seen,
 
   high_block_linenum = high_function_linenum = last_linenum;
 
-  if (flag_sanitize & SANITIZE_ADDRESS)
-    asan_function_start ();
-
   rtx_insn *first = *firstp;
   if (in_initial_view_p (first))
     {
diff --git a/gcc/output.h b/gcc/output.h
index 76cfd58c1e6..bfdecc5ea74 100644
--- a/gcc/output.h
+++ b/gcc/output.h
@@ -178,6 +178,10 @@  extern void assemble_asm (tree);
 /* Get the function's name from a decl, as described by its RTL.  */
 extern const char *get_fnname_from_decl (tree);
 
+/* Output function label, possibly with accompanying metadata.  No additional
+   code or data is output after the label.  */
+extern void assemble_function_label_raw (FILE *, const char *);
+
 /* Output assembler code for the constant pool of a function and associated
    with defining the name of the function.  DECL describes the function.
    NAME is the function's name.  For the constant pool, we use the current
diff --git a/gcc/varasm.cc b/gcc/varasm.cc
index 167aea87091..28c29883df9 100644
--- a/gcc/varasm.cc
+++ b/gcc/varasm.cc
@@ -61,6 +61,7 @@  along with GCC; see the file COPYING3.  If not see
 #include "alloc-pool.h"
 #include "toplev.h"
 #include "opts.h"
+#include "asan.h"
 
 /* The (assembler) name of the first globally-visible object output.  */
 extern GTY(()) const char *first_global_object_name;
@@ -1835,6 +1836,15 @@  get_fnname_from_decl (tree decl)
   return XSTR (x, 0);
 }
 
+void assemble_function_label_raw (FILE *file, const char *name)
+{
+  ASM_OUTPUT_LABEL (file, name);
+  if ((flag_sanitize & SANITIZE_ADDRESS)
+      /* Notify ASAN only about the first function label.  */
+      && (in_cold_section_p == first_function_block_is_cold))
+    asan_function_start ();
+}
+
 /* Output assembler code for the constant pool of a function and associated
    with defining the name of the function.  DECL describes the function.
    NAME is the function's name.  For the constant pool, we use the current