newbie: labels headache

[Alexander Kjeldaas <astor at guardian dot no>]

Hi, I've tried to rewrite the StackGuard patch for egcs but when I
compile the program with -O2, the label guarding the 'call abort'
instruction is removed, which makes the assembler complain.  I'd be
happy if someone could clue me in about my wrongdoings in the
following. This is my first gcc hack so please be gentle :-).

This is what I do in the epilogue code:

(in ix86_expand_epilogue):
[..lots of stuff..]
rts reg = gen_rtx(REG, SImode, 1);
stack_protect_label = gen_label_rtx();
/* documentation suggests this might convince gcc into
   not removing the label, but it doesn't work */
LABEL_PRESERVE_P(stack_protect_label) = 1;
rtx null = GEN_INT(0);
emit_cmp_insn (reg, null, EQ, NULL_RTX, GET_MODE(reg), 0, 0);
emit_jump_insn (gen_bne (stack_protect_label));
[.. here the 'ret' instruction is emitted by egcs ..]
rtx savior = gen_rtx (MEM, FUNCTION_MODE, 
		      gen_rtx(SYMBOL_REF, Pmode, "abort"));
emit_label_after(stack_protect_label, get_insns());   <-- (*) PROBLEM
emit_call_insn (gen_call (savior, GEN_INT(0), NULL_RTX, 0));

The following is my test-case:

void humbug(char *s) {
  char buf[10];
  printf("\"%s\"\n", s);
  strcpy(buf, s);
}

Now, tin the non-optimized case, the code produced does the following:
[gcc -fstack-null-protect -S test.c]

humbug:
        pushl $0
        pushl %ebp
        movl %esp,%ebp
        subl $12,%esp
        movl 12(%ebp),%eax
        pushl %eax
        pushl $.LC0
        call printf
        addl $8,%esp
        movl 12(%ebp),%eax
        pushl %eax
        leal -12(%ebp),%eax
        pushl %eax
        call strcpy
        addl $8,%esp
.L1:
        movl %ebp,%esp
        popl %ebp
        popl %edx          <- canary test
        testl %edx,%edx       ...
        jne .L2               ...
        ret
.L2:
        call abort


Now, with -O2 the following is produced:
[gcc -fstack-null-protect -O2 -S test.c]

humbug:
        pushl $0
        pushl %ebp
        movl %esp,%ebp
        subl $12,%esp
        pushl %ebx
        movl 12(%ebp),%ebx
        pushl %ebx
        pushl $.LC0
        call printf
        pushl %ebx
        leal -12(%ebp),%eax
        pushl %eax
        call strcpy
        movl -16(%ebp),%ebx
        movl %ebp,%esp
        popl %ebp
        popl %edx
        testl %edx,%edx
        jne .L2
        ret                     
        call abort               <-- where's the label???

Here's the patch (relative to 1.0.3a)

diff -ur egcs-1.0.3a/gcc/config/i386/i386.c egcs-1.0.3a.new/gcc/config/i386/i386.c
--- egcs-1.0.3a/gcc/config/i386/i386.c	Tue May  5 02:40:04 1998
+++ egcs-1.0.3a.new/gcc/config/i386/i386.c	Fri Oct 16 02:22:35 1998
@@ -103,6 +103,7 @@
 
 extern FILE *asm_out_file;
 extern char *strcat ();
+extern int stack_protect;
 
 char *singlemove_string ();
 char *output_move_const_single ();
@@ -1815,6 +1816,7 @@
 
 
 
+static int canaryval = 0;
 static rtx pic_label_rtx;
 static char pic_label_name [256];
 static int pic_label_no = 0;
@@ -1855,6 +1857,8 @@
 
 /* Set up the stack and frame (if desired) for the function.  */
 
+#define debug_stack(x...) 
+
 void
 function_prologue (file, size)
      FILE *file;
@@ -1868,6 +1872,15 @@
   long tsize = get_frame_size ();
   int cfa_offset = INCOMING_FRAME_SP_OFFSET, cfa_store_offset = cfa_offset;
 
+  if (stack_protect == 2)
+    {
+      canaryval = (canaryval+4) % 512;
+      xops[0] = GEN_INT(canaryval);
+      output_asm_insn("pushl %0(__canary)", xops);
+    }
+  else if (stack_protect == 1)
+      output_asm_insn ("pushl $0", xops); 
+
   /* pic references don't explicitly mention pic_offset_table_rtx */
   if (TARGET_SCHEDULE_PROLOGUE)
     {
@@ -2131,6 +2144,7 @@
   register int nregs, limit;
   int offset;
   rtx xops[3];
+  rtx stack_protect_label;
   int pic_reg_used = flag_pic && (current_function_uses_pic_offset_table
 				  || current_function_uses_const_pool);
   long tsize = get_frame_size ();
@@ -2238,6 +2252,21 @@
     }
 #endif
 
+  /* canary (stack) protection */
+  if (stack_protect > 0) 
+    { 
+      rtx can, reg = gen_rtx(REG, SImode, 1);
+      stack_protect_label = gen_label_rtx();
+
+      emit_insn (gen_pop(reg));
+      if (stack_protect == 2)
+	can = GEN_INT(canaryval);
+      else
+	can = GEN_INT(0);
+      emit_cmp_insn (reg, can, EQ, NULL_RTX, GET_MODE(reg), 0, 0);
+      emit_jump_insn (gen_bne (stack_protect_label));
+    }
+
   if (current_function_pops_args && current_function_args_size)
     {
       xops[1] = GEN_INT (current_function_pops_args);
@@ -2267,7 +2296,20 @@
     }
   else
 /*    output_asm_insn ("ret", xops);*/
- emit_jump_insn (gen_return_internal ());
+    emit_jump_insn (gen_return_internal ());
+
+  
+  /* canary - put this out-of-line to avoid a mispredicted branch
+     and instruction cache pollution */
+  if (stack_protect > 0)
+    {
+      rtx savior = gen_rtx (MEM, FUNCTION_MODE, 
+			    gen_rtx(SYMBOL_REF, Pmode, "abort"));
+      LABEL_PRESERVE_P(stack_protect_label) = 1;
+      emit_label_after(stack_protect_label, get_insns()); /* PROBLEM!!! */
+      LABEL_PRESERVE_P(stack_protect_label) = 1;
+      emit_call_insn (gen_call (savior, GEN_INT(0), NULL_RTX, 0));
+    }
 }
 
 
diff -ur egcs-1.0.3a/gcc/config/i386/i386.h egcs-1.0.3a.new/gcc/config/i386/i386.h
--- egcs-1.0.3a/gcc/config/i386/i386.h	Fri Dec 19 09:59:33 1997
+++ egcs-1.0.3a.new/gcc/config/i386/i386.h	Fri Oct 16 00:19:40 1998
@@ -924,7 +924,9 @@
 #define PUSH_ROUNDING(BYTES) (((BYTES) + 1) & (-2))
 
 /* Offset of first parameter from the argument pointer register value.  */
-#define FIRST_PARM_OFFSET(FNDECL) 0
+/* IMMUNIX: if neccessary, allow for space of the canaryvalue, or a
+   null word. */
+#define FIRST_PARM_OFFSET(FNDECL) ((stack_protect == 0) ? 0 : 4)
 
 /* Value is the number of bytes of arguments automatically
    popped when returning from a subroutine call.
diff -ur egcs-1.0.3a/gcc/function.c egcs-1.0.3a.new/gcc/function.c
--- egcs-1.0.3a/gcc/function.c	Mon Feb  9 00:01:35 1998
+++ egcs-1.0.3a.new/gcc/function.c	Fri Oct 16 00:19:40 1998
@@ -2580,6 +2580,7 @@
      rtx insns;
 {
   rtx insn;
+  extern int stack_protect;
 
   /* Compute the offsets to use for this function.  */
   in_arg_offset = FIRST_PARM_OFFSET (fndecl);
diff -ur egcs-1.0.3a/gcc/toplev.c egcs-1.0.3a.new/gcc/toplev.c
--- egcs-1.0.3a/gcc/toplev.c	Mon Feb  9 01:56:08 1998
+++ egcs-1.0.3a.new/gcc/toplev.c	Fri Oct 16 00:19:40 1998
@@ -296,6 +296,10 @@
 /* Flag to output bytecode instead of native assembler */
 int output_bytecode = 0;
 
+/* Flag to add IMMUNIX StackGuard canary code to function prologues
+   and epilogues, or null-word protection which is slightly faster. */
+int stack_protect = 0;
+
 /* Pointer to function to compute the name to use to print a declaration.
    DECL is the declaration in question.
    VERBOSITY determines what information will be printed:
@@ -747,7 +751,9 @@
   {"argument-noalias", &flag_argument_noalias, 1},
   {"argument-noalias-global", &flag_argument_noalias, 2},
   {"check-memory-usage", &flag_check_memory_usage, 1},
-  {"prefix-function-name", &flag_prefix_function_name, 1}
+  {"prefix-function-name", &flag_prefix_function_name, 1},
+  {"stack-canary-protect", &stack_protect, 2},
+  {"stack-null-protect", &stack_protect, 1}
 };
 
 /* Table of language-specific options.  */



astor

-- 
 Alexander Kjeldaas, Guardian Networks AS, Trondheim, Norway
 http://www.guardian.no/