public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH 0/2] asan: Align .LASANPC on function boundary
@ 2023-12-07 12:08 Ilya Leoshkevich
  2023-12-07 12:08 ` [PATCH 1/2] Implement ASM_DECLARE_FUNCTION_NAME using ASM_OUTPUT_FUNCTION_LABEL Ilya Leoshkevich
  2023-12-07 12:08 ` [PATCH 2/2] asan: Align .LASANPC on function boundary Ilya Leoshkevich
  0 siblings, 2 replies; 4+ messages in thread
From: Ilya Leoshkevich @ 2023-12-07 12:08 UTC (permalink / raw)
  To: Jakub Jelinek, Jeff Law, Richard Sandiford
  Cc: Andreas Krebbel, gcc-patches, Segher Boessenkool,
	Stefan Schulze Frielinghaus, Ilya Leoshkevich

Hi,

this is another attempt to fix the .LASANPC alignment on s390x.
Currently it's not only inefficient ([1]-[5]), but also causes linker
errors in template-heavy code ([6]).

The previous attempts to add a new constant for minimum code alignment
value ([1]-[5]) did not arouse considerable enthusiasm, and fixing the
fallout ([6]) is probably just a wrong thing to do.

So here I'm taking another approach: making sure that .LASANPC is
aligned on function boundary in the first place.  This requires moving
the asan_function_start() invocation to ASM_OUTPUT_FUNCTION_LABEL().

Bootstrapped and regtested on x86_64-redhat-linux, ppc64le-redhat-linux
and s390x-redhat-linux.  Compile tested for platforms listed in [7].

Best regards,
Ilya

[1] https://gcc.gnu.org/pipermail/gcc-patches/2019-July/525016.html
[2] https://gcc.gnu.org/pipermail/gcc-patches/2019-July/525069.html
[3] https://gcc.gnu.org/pipermail/gcc-patches/2020-June/548338.html
[4] https://gcc.gnu.org/pipermail/gcc-patches/2020-July/549252.html
[5] https://patchwork.ozlabs.org/project/gcc/list/?series=320223
[6] https://patchwork.ozlabs.org/project/gcc/list/?series=297132
[7] http://toolchain.lug-owl.de/laminar/jobs

Ilya Leoshkevich (2):
  Implement ASM_DECLARE_FUNCTION_NAME using ASM_OUTPUT_FUNCTION_LABEL
  asan: Align .LASANPC on function boundary

 gcc/asan.cc                         |  6 ++----
 gcc/config/aarch64/aarch64.cc       |  2 +-
 gcc/config/alpha/alpha.cc           |  5 ++---
 gcc/config/arm/aout.h               |  2 +-
 gcc/config/arm/arm.cc               |  2 +-
 gcc/config/bfin/bfin.h              | 16 ++++++++--------
 gcc/config/c6x/c6x.h                |  2 +-
 gcc/config/gcn/gcn.cc               |  5 ++---
 gcc/config/h8300/h8300.h            |  2 +-
 gcc/config/i386/i386.cc             |  2 +-
 gcc/config/ia64/ia64.cc             |  5 ++---
 gcc/config/mcore/mcore-elf.h        |  2 +-
 gcc/config/microblaze/microblaze.cc |  3 +--
 gcc/config/mips/mips.cc             | 19 ++++++++++---------
 gcc/config/pa/pa.cc                 |  3 ++-
 gcc/config/riscv/riscv.cc           |  2 +-
 gcc/config/rs6000/rs6000.cc         |  4 ++--
 gcc/config/s390/s390.cc             |  2 +-
 gcc/defaults.h                      |  2 +-
 gcc/final.cc                        |  3 ---
 gcc/output.h                        |  4 ++++
 gcc/varasm.cc                       | 10 ++++++++++
 22 files changed, 55 insertions(+), 48 deletions(-)

-- 
2.43.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 1/2] Implement ASM_DECLARE_FUNCTION_NAME using ASM_OUTPUT_FUNCTION_LABEL
  2023-12-07 12:08 [PATCH 0/2] asan: Align .LASANPC on function boundary Ilya Leoshkevich
@ 2023-12-07 12:08 ` Ilya Leoshkevich
  2023-12-07 12:08 ` [PATCH 2/2] asan: Align .LASANPC on function boundary Ilya Leoshkevich
  1 sibling, 0 replies; 4+ messages in thread
From: Ilya Leoshkevich @ 2023-12-07 12:08 UTC (permalink / raw)
  To: Jakub Jelinek, Jeff Law, Richard Sandiford
  Cc: Andreas Krebbel, gcc-patches, Segher Boessenkool,
	Stefan Schulze Frielinghaus, Ilya Leoshkevich

gccint recommends using ASM_OUTPUT_FUNCTION_LABEL in
ASM_DECLARE_FUNCTION_NAME, but many implementations use
ASM_OUTPUT_LABEL instead.  It's inconsistent and prevents changes to
ASM_OUTPUT_FUNCTION_LABEL from affecting the respective targets.
---
 gcc/config/aarch64/aarch64.cc       |  2 +-
 gcc/config/alpha/alpha.cc           |  5 ++---
 gcc/config/arm/aout.h               |  2 +-
 gcc/config/arm/arm.cc               |  2 +-
 gcc/config/bfin/bfin.h              | 16 ++++++++--------
 gcc/config/c6x/c6x.h                |  2 +-
 gcc/config/gcn/gcn.cc               |  5 ++---
 gcc/config/h8300/h8300.h            |  2 +-
 gcc/config/ia64/ia64.cc             |  5 ++---
 gcc/config/mcore/mcore-elf.h        |  2 +-
 gcc/config/microblaze/microblaze.cc |  3 +--
 gcc/config/mips/mips.cc             | 19 ++++++++++---------
 gcc/config/pa/pa.cc                 |  3 ++-
 gcc/config/riscv/riscv.cc           |  2 +-
 gcc/config/rs6000/rs6000.cc         |  4 ++--
 15 files changed, 36 insertions(+), 38 deletions(-)

diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
index 8f50a70083d..bf247a8fd17 100644
--- a/gcc/config/aarch64/aarch64.cc
+++ b/gcc/config/aarch64/aarch64.cc
@@ -23285,7 +23285,7 @@ aarch64_declare_function_name (FILE *stream, const char* name,
 
   /* Don't forget the type directive for ELF.  */
   ASM_OUTPUT_TYPE_DIRECTIVE (stream, name, "function");
-  ASM_OUTPUT_LABEL (stream, name);
+  ASM_OUTPUT_FUNCTION_LABEL (stream, name, fndecl);
 
   cfun->machine->label_is_assembled = true;
 }
diff --git a/gcc/config/alpha/alpha.cc b/gcc/config/alpha/alpha.cc
index 6aa93783226..8118255e737 100644
--- a/gcc/config/alpha/alpha.cc
+++ b/gcc/config/alpha/alpha.cc
@@ -7986,8 +7986,7 @@ int num_source_filenames = 0;
 /* Output the textual info surrounding the prologue.  */
 
 void
-alpha_start_function (FILE *file, const char *fnname,
-		      tree decl ATTRIBUTE_UNUSED)
+alpha_start_function (FILE *file, const char *fnname, tree decl)
 {
   unsigned long imask, fmask;
   /* Complete stack size needed.  */
@@ -8052,7 +8051,7 @@ alpha_start_function (FILE *file, const char *fnname,
   if (TARGET_ABI_OPEN_VMS)
     strcat (entry_label, "..en");
 
-  ASM_OUTPUT_LABEL (file, entry_label);
+  ASM_OUTPUT_FUNCTION_LABEL (file, entry_label, decl);
   inside_function = TRUE;
 
   if (TARGET_ABI_OPEN_VMS)
diff --git a/gcc/config/arm/aout.h b/gcc/config/arm/aout.h
index 49896bb9620..380147aed7d 100644
--- a/gcc/config/arm/aout.h
+++ b/gcc/config/arm/aout.h
@@ -152,7 +152,7 @@
   do							\
     {							\
       ARM_DECLARE_FUNCTION_NAME (STREAM, NAME, DECL);   \
-      ASM_OUTPUT_LABEL (STREAM, NAME);			\
+      ASM_OUTPUT_FUNCTION_LABEL (STREAM, NAME, DECL);	\
     }							\
   while (0)
 #endif
diff --git a/gcc/config/arm/arm.cc b/gcc/config/arm/arm.cc
index 6e3e2e8fb1b..7fd9bc19882 100644
--- a/gcc/config/arm/arm.cc
+++ b/gcc/config/arm/arm.cc
@@ -21801,7 +21801,7 @@ arm_asm_declare_function_name (FILE *file, const char *name, tree decl)
   ARM_DECLARE_FUNCTION_NAME (file, name, decl);
   ASM_OUTPUT_TYPE_DIRECTIVE (file, name, "function");
   ASM_DECLARE_RESULT (file, DECL_RESULT (decl));
-  ASM_OUTPUT_LABEL (file, name);
+  ASM_OUTPUT_FUNCTION_LABEL (file, name, decl);
 
   if (cmse_name)
     ASM_OUTPUT_LABEL (file, cmse_name);
diff --git a/gcc/config/bfin/bfin.h b/gcc/config/bfin/bfin.h
index c25f41f6839..60a8d716819 100644
--- a/gcc/config/bfin/bfin.h
+++ b/gcc/config/bfin/bfin.h
@@ -995,14 +995,14 @@ typedef enum directives {
         fputc ('\n',FILE);			\
       } while (0)
 
-#define ASM_DECLARE_FUNCTION_NAME(FILE,NAME,DECL) \
-  do {					\
-    fputs (".type ", FILE);           	\
-    assemble_name (FILE, NAME);         \
-    fputs (", STT_FUNC", FILE);         \
-    fputc (';',FILE);                   \
-    fputc ('\n',FILE);			\
-    ASM_OUTPUT_LABEL(FILE, NAME);	\
+#define ASM_DECLARE_FUNCTION_NAME(FILE, NAME, DECL)	\
+  do {							\
+    fputs (".type ", FILE);				\
+    assemble_name (FILE, NAME);				\
+    fputs (", STT_FUNC", FILE);				\
+    fputc (';', FILE);					\
+    fputc ('\n', FILE);					\
+    ASM_OUTPUT_FUNCTION_LABEL (FILE, NAME, DECL);	\
   } while (0)
 
 #define ASM_OUTPUT_LABEL(FILE, NAME)    \
diff --git a/gcc/config/c6x/c6x.h b/gcc/config/c6x/c6x.h
index 26b2f2f0700..790b9627ebe 100644
--- a/gcc/config/c6x/c6x.h
+++ b/gcc/config/c6x/c6x.h
@@ -459,7 +459,7 @@ struct GTY(()) machine_function
       c6x_output_file_unwind (FILE);				\
       ASM_OUTPUT_TYPE_DIRECTIVE (FILE, NAME, "function");	\
       ASM_DECLARE_RESULT (FILE, DECL_RESULT (DECL));		\
-      ASM_OUTPUT_LABEL (FILE, NAME);				\
+      ASM_OUTPUT_FUNCTION_LABEL (FILE, NAME, DECL);		\
     }								\
   while (0)
 
diff --git a/gcc/config/gcn/gcn.cc b/gcc/config/gcn/gcn.cc
index 031b405e810..8dfbf903a04 100644
--- a/gcc/config/gcn/gcn.cc
+++ b/gcc/config/gcn/gcn.cc
@@ -6523,7 +6523,7 @@ output_file_start (void)
    comments that pass information to mkoffload.  */
 
 void
-gcn_hsa_declare_function_name (FILE *file, const char *name, tree)
+gcn_hsa_declare_function_name (FILE *file, const char *name, tree decl)
 {
   int sgpr, vgpr, avgpr;
   bool xnack_enabled = TARGET_XNACK;
@@ -6684,8 +6684,7 @@ gcn_hsa_declare_function_name (FILE *file, const char *name, tree)
   fputs ("\t.type\t", file);
   assemble_name (file, name);
   fputs (",@function\n", file);
-  assemble_name (file, name);
-  fputs (":\n", file);
+  ASM_OUTPUT_FUNCTION_LABEL (file, name, decl);
 
   /* This comment is read by mkoffload.  */
   if (flag_openacc)
diff --git a/gcc/config/h8300/h8300.h b/gcc/config/h8300/h8300.h
index 311e4023dfd..b62779a9273 100644
--- a/gcc/config/h8300/h8300.h
+++ b/gcc/config/h8300/h8300.h
@@ -650,7 +650,7 @@ struct cum_arg
 #define GLOBAL_ASM_OP "\t.global "
 
 #define ASM_DECLARE_FUNCTION_NAME(FILE, NAME, DECL) \
-   ASM_OUTPUT_LABEL (FILE, NAME)
+   ASM_OUTPUT_FUNCTION_LABEL (FILE, NAME, DECL)
 
 /* This is how to store into the string LABEL
    the symbol_ref name of an internal numbered label where
diff --git a/gcc/config/ia64/ia64.cc b/gcc/config/ia64/ia64.cc
index ac566efcf19..92d00bf922f 100644
--- a/gcc/config/ia64/ia64.cc
+++ b/gcc/config/ia64/ia64.cc
@@ -3886,8 +3886,7 @@ ia64_expand_prologue (void)
 /* Output the textual info surrounding the prologue.  */
 
 void
-ia64_start_function (FILE *file, const char *fnname,
-		     tree decl ATTRIBUTE_UNUSED)
+ia64_start_function (FILE *file, const char *fnname, tree decl)
 {
 #if TARGET_ABI_OPEN_VMS
   vms_start_function (fnname);
@@ -3896,7 +3895,7 @@ ia64_start_function (FILE *file, const char *fnname,
   fputs ("\t.proc ", file);
   assemble_name (file, fnname);
   fputc ('\n', file);
-  ASM_OUTPUT_LABEL (file, fnname);
+  ASM_OUTPUT_FUNCTION_LABEL (file, fnname, decl);
 }
 
 /* Called after register allocation to add any instructions needed for the
diff --git a/gcc/config/mcore/mcore-elf.h b/gcc/config/mcore/mcore-elf.h
index bf1b093d327..3e0b903727b 100644
--- a/gcc/config/mcore/mcore-elf.h
+++ b/gcc/config/mcore/mcore-elf.h
@@ -51,7 +51,7 @@ along with GCC; see the file COPYING3.  If not see
 	}							\
       ASM_OUTPUT_TYPE_DIRECTIVE (FILE, NAME, "function");	\
       ASM_DECLARE_RESULT (FILE, DECL_RESULT (DECL));		\
-      ASM_OUTPUT_LABEL (FILE, NAME);				\
+      ASM_OUTPUT_FUNCTION_LABEL (FILE, NAME, DECL);		\
     }								\
   while (0)
 
diff --git a/gcc/config/microblaze/microblaze.cc b/gcc/config/microblaze/microblaze.cc
index 3ea177b835e..79405a32dc1 100644
--- a/gcc/config/microblaze/microblaze.cc
+++ b/gcc/config/microblaze/microblaze.cc
@@ -2792,8 +2792,7 @@ microblaze_function_prologue (FILE * file)
 	ASM_OUTPUT_TYPE_DIRECTIVE (file, fnname, "function");
     }
 
-  assemble_name (file, fnname);
-  fputs (":\n", file);
+  ASM_OUTPUT_FUNCTION_LABEL (file, fnname, current_function_decl);
 
   if (interrupt_handler && strcmp (INTERRUPT_HANDLER_NAME, fnname))
     fputs ("_interrupt_handler:\n", file);
diff --git a/gcc/config/mips/mips.cc b/gcc/config/mips/mips.cc
index 9180dbbf843..be5302b0b7f 100644
--- a/gcc/config/mips/mips.cc
+++ b/gcc/config/mips/mips.cc
@@ -7269,7 +7269,7 @@ mips_gimplify_va_arg_expr (tree valist, tree type, gimple_seq *pre_p,
 /* Declare a unique, locally-binding function called NAME, then start
    its definition.  */
 
-static void
+static tree
 mips_start_unique_function (const char *name)
 {
   tree decl;
@@ -7291,13 +7291,15 @@ mips_start_unique_function (const char *name)
   fputs ("\t.hidden\t", asm_out_file);
   assemble_name (asm_out_file, name);
   putc ('\n', asm_out_file);
+
+  return decl;
 }
 
 /* Start a definition of function NAME.  MIPS16_P indicates whether the
    function contains MIPS16 code.  */
 
 static void
-mips_start_function_definition (const char *name, bool mips16_p)
+mips_start_function_definition (const char *name, bool mips16_p, tree decl)
 {
   if (mips16_p)
     fprintf (asm_out_file, "\t.set\tmips16\n");
@@ -7321,8 +7323,7 @@ mips_start_function_definition (const char *name, bool mips16_p)
   ASM_OUTPUT_TYPE_DIRECTIVE (asm_out_file, name, "function");
 
   /* Start the definition proper.  */
-  assemble_name (asm_out_file, name);
-  fputs (":\n", asm_out_file);
+  ASM_OUTPUT_FUNCTION_LABEL (asm_out_file, name, decl);
 }
 
 /* End a function definition started by mips_start_function_definition.  */
@@ -7349,8 +7350,8 @@ mips_finish_stub (mips_one_only_stub **stub_ptr)
     return;
 
   const char *name = stub->get_name ();
-  mips_start_unique_function (name);
-  mips_start_function_definition (name, false);
+  tree decl = mips_start_unique_function (name);
+  mips_start_function_definition (name, false, decl);
   stub->output_body ();
   mips_end_function_definition (name);
   delete stub;
@@ -7604,7 +7605,7 @@ mips16_build_function_stub (void)
 
   /* Start the function definition.  */
   assemble_start_function (stubdecl, stubname);
-  mips_start_function_definition (stubname, false);
+  mips_start_function_definition (stubname, false, stubdecl);
 
   /* If generating pic2 code, either set up the global pointer or
      switch to pic0.  */
@@ -7864,7 +7865,7 @@ mips16_build_call_stub (rtx retval, rtx *fn_ptr, rtx args_size, int fp_code)
 
       /* Start the function definition.  */
       assemble_start_function (stubdecl, stubname);
-      mips_start_function_definition (stubname, false);
+      mips_start_function_definition (stubname, false, stubdecl);
 
       if (fp_ret_p)
 	{
@@ -12064,7 +12065,7 @@ mips_output_function_prologue (FILE *file)
      assemble_start_function.  This is needed so that the name used here
      exactly matches the name used in ASM_DECLARE_FUNCTION_NAME.  */
   fnname = XSTR (XEXP (DECL_RTL (current_function_decl), 0), 0);
-  mips_start_function_definition (fnname, TARGET_MIPS16);
+  mips_start_function_definition (fnname, TARGET_MIPS16, current_function_decl);
 
   /* Output MIPS-specific frame information.  */
   if (!flag_inhibit_size_directive)
diff --git a/gcc/config/pa/pa.cc b/gcc/config/pa/pa.cc
index 2ee987796f6..cd9210a3ff9 100644
--- a/gcc/config/pa/pa.cc
+++ b/gcc/config/pa/pa.cc
@@ -3991,7 +3991,8 @@ pa_output_function_label (FILE *file)
   /* The function's label and associated .PROC must never be
      separated and must be output *after* any profiling declarations
      to avoid changing spaces/subspaces within a procedure.  */
-  ASM_OUTPUT_LABEL (file, XSTR (XEXP (DECL_RTL (current_function_decl), 0), 0));
+  const char *name = XSTR (XEXP (DECL_RTL (current_function_decl), 0), 0);
+  ASM_OUTPUT_FUNCTION_LABEL (file, name, current_function_decl);
   fputs ("\t.PROC\n", file);
 
   /* pa_expand_prologue does the dirty work now.  We just need
diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
index 3f111fa0393..73cf9361c60 100644
--- a/gcc/config/riscv/riscv.cc
+++ b/gcc/config/riscv/riscv.cc
@@ -8366,7 +8366,7 @@ riscv_declare_function_name (FILE *stream, const char *name, tree fndecl)
 {
   riscv_asm_output_variant_cc (stream, fndecl, name);
   ASM_OUTPUT_TYPE_DIRECTIVE (stream, name, "function");
-  ASM_OUTPUT_LABEL (stream, name);
+  ASM_OUTPUT_FUNCTION_LABEL (stream, name, fndecl);
   if (DECL_FUNCTION_SPECIFIC_TARGET (fndecl))
     {
       fprintf (stream, "\t.option push\n");
diff --git a/gcc/config/rs6000/rs6000.cc b/gcc/config/rs6000/rs6000.cc
index 2d8afc12119..aa6b1ec29ee 100644
--- a/gcc/config/rs6000/rs6000.cc
+++ b/gcc/config/rs6000/rs6000.cc
@@ -21400,7 +21400,7 @@ rs6000_elf_declare_function_name (FILE *file, const char *name, tree decl)
       fputs ("\t.long 0\n", file);
       fprintf (file, "\t.previous\n");
     }
-  ASM_OUTPUT_LABEL (file, name);
+  ASM_OUTPUT_FUNCTION_LABEL (file, name, decl);
 }
 
 static void rs6000_elf_file_end (void) ATTRIBUTE_UNUSED;
@@ -21973,7 +21973,7 @@ rs6000_xcoff_declare_function_name (FILE *file, const char *name, tree decl)
   assemble_name (file, buffer);
   fputs (TARGET_32BIT ? "\n" : ",3\n", file);
 
-  ASM_OUTPUT_LABEL (file, buffer);
+  ASM_OUTPUT_FUNCTION_LABEL (file, buffer, decl);
 
   symtab_node::get (decl)->call_for_symbol_and_aliases (rs6000_declare_alias,
 							&data, true);
-- 
2.43.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 2/2] asan: Align .LASANPC on function boundary
  2023-12-07 12:08 [PATCH 0/2] asan: Align .LASANPC on function boundary Ilya Leoshkevich
  2023-12-07 12:08 ` [PATCH 1/2] Implement ASM_DECLARE_FUNCTION_NAME using ASM_OUTPUT_FUNCTION_LABEL Ilya Leoshkevich
@ 2023-12-07 12:08 ` Ilya Leoshkevich
  2023-12-07 12:25   ` Jakub Jelinek
  1 sibling, 1 reply; 4+ messages in thread
From: Ilya Leoshkevich @ 2023-12-07 12:08 UTC (permalink / raw)
  To: Jakub Jelinek, Jeff Law, Richard Sandiford
  Cc: Andreas Krebbel, gcc-patches, Segher Boessenkool,
	Stefan Schulze Frielinghaus, Ilya Leoshkevich

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(-)

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
-- 
2.43.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 2/2] asan: Align .LASANPC on function boundary
  2023-12-07 12:08 ` [PATCH 2/2] asan: Align .LASANPC on function boundary Ilya Leoshkevich
@ 2023-12-07 12:25   ` Jakub Jelinek
  0 siblings, 0 replies; 4+ messages in thread
From: Jakub Jelinek @ 2023-12-07 12:25 UTC (permalink / raw)
  To: Ilya Leoshkevich
  Cc: Jeff Law, Richard Sandiford, Andreas Krebbel, gcc-patches,
	Segher Boessenkool, Stefan Schulze Frielinghaus

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


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2023-12-07 12:25 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-12-07 12:08 [PATCH 0/2] asan: Align .LASANPC on function boundary Ilya Leoshkevich
2023-12-07 12:08 ` [PATCH 1/2] Implement ASM_DECLARE_FUNCTION_NAME using ASM_OUTPUT_FUNCTION_LABEL Ilya Leoshkevich
2023-12-07 12:08 ` [PATCH 2/2] asan: Align .LASANPC on function boundary Ilya Leoshkevich
2023-12-07 12:25   ` Jakub Jelinek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).