* [patch, arm] Skip insns generated by -fstack-protector
@ 2010-12-09 13:45 Yao Qi
2010-12-20 14:08 ` Yao Qi
2010-12-23 4:16 ` Joel Brobecker
0 siblings, 2 replies; 6+ messages in thread
From: Yao Qi @ 2010-12-09 13:45 UTC (permalink / raw)
To: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 671 bytes --]
There are some testcase failures when -fstack-protector is turned on
during testcase compilation, reported here,
https://bugs.launchpad.net/gdb-linaro/+bug/616000
This bug is found on both arm and i386, but has to be fixed respectively.
This patch is to address this problem by matching insn pattern (movw,
movt, ldr, and str), and expand post_prologue_pc to skip insns for stack
protector. Existing {arm,thumb}_analyze_prologue has capability to skip
these insns, so they are not changed.
Regression tested with combination of
"\{-mthumb,-marm\}\{-fstack-protector,-fno-stack-protector}\{-march=armv7-a,-march=armv5t\}".
OK for mainline?
--
Yao (é½å°§)
[-- Attachment #2: arm_skip_stack_protector.patch --]
[-- Type: text/x-patch, Size: 8228 bytes --]
gdb/
* arm-tdep.c (thumb_analyze_prologue): Move some code ...
(EXTRACT_MOVW_MOVT_IMM_T): ... here. New macro.
(EXTRACT_MOVW_MOVT_IMM_A): New macro.
(arm_analyze_load_stack_chk_guard): New.
(arm_skip_stack_protector): New.
(arm_skip_prologue): Adjust post_prologue_pc by arm_skip_stack_protector.
diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index 636c1de..678d44c 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -495,6 +495,18 @@ skip_prologue_function (CORE_ADDR pc)
#define BranchDest(addr,instr) \
((CORE_ADDR) (((long) (addr)) + 8 + (sbits (instr, 0, 23) << 2)))
+/* Extract the immediate from instruction movw/movt of encoding T. INSN1 is the
+ first 16-bit of instruction, and INSN2 is the second 16-bit of
+ instruction. */
+#define EXTRACT_MOVW_MOVT_IMM_T(insn1, insn2) \
+ ((bits ((insn1), 0, 3) << 12) | (bits ((insn1), 10, 10) << 11)\
+ | (bits ((insn2), 12, 14) << 8) | bits ((insn2), 0, 7))
+
+/* Extract the immediate form instruction movw/movt of encoding A. INSN is
+ the 32-bit instruction. */
+#define EXTRACT_MOVW_MOVT_IMM_A(insn) \
+ ((bits ((insn), 16, 19) << 12) | bits ((insn), 0, 11))
+
/* Decode immediate value; implements ThumbExpandImmediate pseudo-op. */
static unsigned int
@@ -1003,10 +1015,8 @@ thumb_analyze_prologue (struct gdbarch *gdbarch,
else if ((insn & 0xfbf0) == 0xf240) /* movw Rd, #const */
{
- unsigned int imm = ((bits (insn, 0, 3) << 12)
- | (bits (insn, 10, 10) << 11)
- | (bits (inst2, 12, 14) << 8)
- | bits (inst2, 0, 7));
+ unsigned int imm
+ = EXTRACT_MOVW_MOVT_IMM_T (insn, inst2);
regs[bits (inst2, 8, 11)] = pv_constant (imm);
}
@@ -1129,6 +1139,183 @@ thumb_analyze_prologue (struct gdbarch *gdbarch,
return unrecognized_pc;
}
+
+/* Try to analyze the the instructions starting from PC, which load symbol
+ __stack_chk_guard. Return the address of instruction after loading this
+ symbol, set the dest register number to *BASEREG, and set the size of
+ instructions for loading symbol in OFFSET. Return 0 if instructions are
+ not recognized. */
+
+static CORE_ADDR
+arm_analyze_load_stack_chk_guard(CORE_ADDR pc, struct gdbarch *gdbarch,
+ unsigned int *destreg, int *offset)
+{
+ enum bfd_endian byte_order_for_code = gdbarch_byte_order_for_code (gdbarch);
+ int is_thumb = arm_pc_is_thumb (gdbarch, pc);
+ unsigned int low, high, address;
+
+ address = 0;
+ if (is_thumb)
+ {
+ unsigned short insn1
+ = read_memory_unsigned_integer (pc, 2, byte_order_for_code);
+
+ if ((insn1 & 0xf800) == 0x4800) /* ldr Rd, #immed */
+ {
+ *destreg = bits (insn1, 8, 10);
+ *offset = 2;
+ address = bits (insn1, 0, 7);
+ }
+ else if ((insn1 & 0xfbf0) == 0xf240) /* movw Rd, #const */
+ {
+ unsigned short insn2
+ = read_memory_unsigned_integer (pc + 2, 2, byte_order_for_code);
+ low = EXTRACT_MOVW_MOVT_IMM_T (insn1, insn2);
+
+ insn1
+ = read_memory_unsigned_integer (pc + 4, 2, byte_order_for_code);
+ insn2
+ = read_memory_unsigned_integer (pc + 6, 2, byte_order_for_code);
+
+ /* movt Rd, #const */
+ if ((insn1 & 0xfbc0) == 0xf2c0)
+ {
+ high = EXTRACT_MOVW_MOVT_IMM_T (insn1, insn2);
+ *destreg = bits (insn2, 8, 11);
+ *offset = 8;
+ address = (high << 16 | low);
+ }
+ }
+ }
+ else
+ {
+ unsigned int insn
+ = read_memory_unsigned_integer (pc, 4, byte_order_for_code);
+ if ((insn & 0x0e5f0000) == 0x041f0000) /* ldr Rd, #immed */
+ {
+ address = bits (insn, 0, 11);
+ *destreg = bits (insn, 12, 15);
+ *offset = 4;
+ }
+ else if ((insn & 0x0ff00000) == 0x03000000) /* movw Rd, #const */
+ {
+ low = EXTRACT_MOVW_MOVT_IMM_A (insn);
+
+ insn
+ = read_memory_unsigned_integer (pc + 4, 4, byte_order_for_code);
+
+ if ((insn & 0x0ff00000) == 0x03400000) /* movt Rd, #const */
+ high = EXTRACT_MOVW_MOVT_IMM_A (insn);
+
+ address = (high << 16 | low);
+ *destreg = bits (insn, 12, 15);
+ *offset = 8;
+ }
+ }
+
+ return address;
+}
+
+/* Try to skip a sequence of instructions used for stack protector. If PC
+ points to the first instruction of this sequence, return the address of first
+ instruction after this sequence, otherwise, return original PC.
+
+ On arm, this sequence of instructions is composed by mainly three steps,
+ Step 1: load symbol __stack_chk_guard,
+ Step 2: load from address of __stack_chk_guard,
+ Step 3: store it to somewhere else.
+
+ Usually, instructions on step 2 and step 3 are the same on various ARM
+ archtectures. On step 2, it is one instruction 'ldr Rx, [Rn, #0]', and
+ on step 3, it is also one instruction 'str Rx, [r7, #immd]'. However,
+ instructions in step 1 varies from different ARM archs. On ARMv7, they are,
+
+ movw Rn, #:lower16:__stack_chk_guard
+ movt Rn, #:upper16:__stack_chk_guard
+
+ On ARMv5t, it is,
+
+ ldr Rn, .Label
+ ....
+ .Lable:
+ .word __stack_chk_guard
+
+ Since ldr/str is a very popular instruction, we can't use them as
+ 'fingerprint' or 'signature' of stack protector sequence. Here we choose
+ sequence {movw/movt, ldr}/ldr/str plus symbol __stack_chk_guard, if not
+ stripped, as 'fingerprint' of stack protector cdoe sequence. */
+
+static CORE_ADDR
+arm_skip_stack_protector(CORE_ADDR pc, struct gdbarch *gdbarch)
+{
+ enum bfd_endian byte_order_for_code = gdbarch_byte_order_for_code (gdbarch);
+ unsigned int address, basereg;
+ struct minimal_symbol *stack_chk_guard;
+ int offset;
+ int is_thumb = arm_pc_is_thumb (gdbarch, pc);
+
+ /* Try to parse the instructions in Step 1. */
+ CORE_ADDR addr = arm_analyze_load_stack_chk_guard (pc, gdbarch,
+ &basereg, &offset);
+ if (!addr)
+ return pc;
+
+ stack_chk_guard = lookup_minimal_symbol_by_pc (addr);
+ /* If name of symbol doesn't start with '__stack_chk_guard', this
+ instruction sequence is not for stack protector. If symbol is
+ removed, we conservatively think this sequence is for stack protector. */
+ if (stack_chk_guard
+ && strncmp (SYMBOL_NATURAL_NAME(stack_chk_guard), "__stack_chk_guard",
+ strlen ("__stack_chk_guard")))
+ return pc;
+
+ if (is_thumb)
+ {
+ unsigned int destreg;
+ unsigned short insn
+ = read_memory_unsigned_integer (pc + offset, 2, byte_order_for_code);
+ /* Step 2: ldr Rd, [Rn, #immed], encoding T1. */
+ if ((insn & 0xf800) != 0x6800)
+ return pc;
+ if (bits (insn, 3, 5) != basereg)
+ return pc;
+ destreg = bits (insn, 0, 2);
+
+ insn = read_memory_unsigned_integer (pc + offset + 2, 2,
+ byte_order_for_code);
+ /* Step 3: str Rd, [Rn, #immed], encoding T1. */
+ if ((insn & 0xf800) != 0x6000)
+ return pc;
+ if (destreg != bits (insn, 0, 2))
+ return pc;
+ }
+ else
+ {
+ unsigned int destreg;
+ unsigned int insn
+ = read_memory_unsigned_integer (pc + offset, 4, byte_order_for_code);
+ /* Step 2: ldr Rd, [Rn, #immed], encoding A1. */
+ if ((insn & 0x0e500000) != 0x04100000)
+ return pc;
+ if (bits (insn, 16, 19) != basereg)
+ return pc;
+ destreg = bits (insn, 12, 15);
+ /* Step 3: str Rd, [Rn, #immed], encoding A1. */
+ insn = read_memory_unsigned_integer (pc + offset + 4,
+ 4, byte_order_for_code);
+ if ((insn & 0x0e500000) != 0x04000000)
+ return pc;
+ if (bits (insn, 12, 15) != destreg)
+ return pc;
+ }
+ /* The size of total two instructions ldr/str is 4 on Thumb-2, while 8
+ on arm. */
+ if (is_thumb)
+ return pc + offset + 4;
+ else
+ return pc + offset + 8;
+}
+
/* Advance the PC across any function entry prologue instructions to
reach some "real" code.
@@ -1162,6 +1349,12 @@ arm_skip_prologue (struct gdbarch *gdbarch, CORE_ADDR pc)
= skip_prologue_using_sal (gdbarch, func_addr);
struct symtab *s = find_pc_symtab (func_addr);
+ if (post_prologue_pc)
+ {
+ post_prologue_pc
+ = arm_skip_stack_protector (post_prologue_pc, gdbarch);
+ }
+
/* GCC always emits a line note before the prologue and another
one after, even if the two are at the same address or on the
same line. Take advantage of this so that we do not need to
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch, arm] Skip insns generated by -fstack-protector
2010-12-09 13:45 [patch, arm] Skip insns generated by -fstack-protector Yao Qi
@ 2010-12-20 14:08 ` Yao Qi
2010-12-23 4:16 ` Joel Brobecker
1 sibling, 0 replies; 6+ messages in thread
From: Yao Qi @ 2010-12-20 14:08 UTC (permalink / raw)
To: gdb-patches
On 12/09/2010 09:45 PM, Yao Qi wrote:
> gdb/
> * arm-tdep.c (thumb_analyze_prologue): Move some code ...
> (EXTRACT_MOVW_MOVT_IMM_T): ... here. New macro.
> (EXTRACT_MOVW_MOVT_IMM_A): New macro.
> (arm_analyze_load_stack_chk_guard): New.
> (arm_skip_stack_protector): New.
> (arm_skip_prologue): Adjust post_prologue_pc by arm_skip_stack_protector.
Ping.
http://sourceware.org/ml/gdb-patches/2010-12/msg00110.html
--
Yao (é½å°§)
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch, arm] Skip insns generated by -fstack-protector
2010-12-09 13:45 [patch, arm] Skip insns generated by -fstack-protector Yao Qi
2010-12-20 14:08 ` Yao Qi
@ 2010-12-23 4:16 ` Joel Brobecker
2010-12-23 9:29 ` Yao Qi
1 sibling, 1 reply; 6+ messages in thread
From: Joel Brobecker @ 2010-12-23 4:16 UTC (permalink / raw)
To: Yao Qi; +Cc: gdb-patches
> gdb/
> * arm-tdep.c (thumb_analyze_prologue): Move some code ...
> (EXTRACT_MOVW_MOVT_IMM_T): ... here. New macro.
> (EXTRACT_MOVW_MOVT_IMM_A): New macro.
> (arm_analyze_load_stack_chk_guard): New.
> (arm_skip_stack_protector): New.
> (arm_skip_prologue): Adjust post_prologue_pc by arm_skip_stack_protector.
Comment below. I cannot really judge the actual correctness of the
code used to scan the prologue, since I don't know arm, but I don't
think that this is important.
The code looks good overall, and the patch is pre-approved after all
comments below are addressed. Be aware that there is one issue below
that I'd like to understand...
> +/* Extract the immediate from instruction movw/movt of encoding T. INSN1 is the
> + first 16-bit of instruction, and INSN2 is the second 16-bit of
> + instruction. */
The first line is a little too long. It's exactly 80 characters, but
I would prefer if it didn't exceed 78. I occasionally break that rule
when the extra 2 characters allow the code formatting to be less ugly,
but this is not the case here.
> +/* Extract the immediate form instruction movw/movt of encoding A. INSN is
^^^^ spelling `from'
> + the 32-bit instruction. */
> +#define EXTRACT_MOVW_MOVT_IMM_A(insn) \
> + ((bits ((insn), 16, 19) << 12) | bits ((insn), 0, 11))
Just on a personal suggestion that you don't have to agree with, I find:
> - unsigned int imm = ((bits (insn, 0, 3) << 12)
> - | (bits (insn, 10, 10) << 11)
> - | (bits (inst2, 12, 14) << 8)
> - | bits (inst2, 0, 7));
... more easily readable than the two-line form you used in the macro.
> }
>
> +
> +/* Try to analyze the the instructions starting from PC, which load symbol
^^^^^^^
> + else
> + {
> + unsigned int insn
> + = read_memory_unsigned_integer (pc, 4, byte_order_for_code);
> + if ((insn & 0x0e5f0000) == 0x041f0000) /* ldr Rd, #immed */
Empty line after local variable declaration.
> +/* Try to skip a sequence of instructions used for stack protector. If PC
> + points to the first instruction of this sequence, return the address of first
> + instruction after this sequence, otherwise, return original PC.
> +
> + On arm, this sequence of instructions is composed by mainly three steps,
^^ of
> + Step 1: load symbol __stack_chk_guard,
> + Step 2: load from address of __stack_chk_guard,
> + Step 3: store it to somewhere else.
> +
> + Usually, instructions on step 2 and step 3 are the same on various ARM
> + archtectures. On step 2, it is one instruction 'ldr Rx, [Rn, #0]', and
> + on step 3, it is also one instruction 'str Rx, [r7, #immd]'. However,
> + instructions in step 1 varies from different ARM archs. On ARMv7, they are,
^^^^^^ vary (plural)
I am not sure that "from different ARM archs" is correct English (arches,
btw). Suggest "from arch to arch" (we know that we're dealing with ARM
only in this file). Perhaps "from ARM arch to arch" is also good English
but I'm not sure.
> + Since ldr/str is a very popular instruction, we can't use them as
> + 'fingerprint' or 'signature' of stack protector sequence. Here we choose
> + sequence {movw/movt, ldr}/ldr/str plus symbol __stack_chk_guard, if not
> + stripped, as 'fingerprint' of stack protector cdoe sequence. */
^^ the ^^ a ^^^^^ code
> +static CORE_ADDR
> +arm_skip_stack_protector(CORE_ADDR pc, struct gdbarch *gdbarch)
> +{
> + enum bfd_endian byte_order_for_code = gdbarch_byte_order_for_code (gdbarch);
[...]
> + int is_thumb = arm_pc_is_thumb (gdbarch, pc);
> +
> + /* Try to parse the instructions in Step 1. */
> + CORE_ADDR addr = arm_analyze_load_stack_chk_guard (pc, gdbarch,
> + &basereg, &offset);
> + if (!addr)
> + return pc;
This is a situation where we set ourselves up for adding code between
the two groups of local variable declarations. We've done that in
the past... I think that compilers detect that, now, so it's not so
bad.
I suggest that you move the declaration of variable `addr' to the
first group of declarations, and then assign it later. Or you'll
also need to add an empty line after its declaration.
> + stack_chk_guard = lookup_minimal_symbol_by_pc (addr);
> + /* If name of symbol doesn't start with '__stack_chk_guard', this
> + instruction sequence is not for stack protector. If symbol is
> + removed, we conservatively think this sequence is for stack protector. */
> + if (stack_chk_guard
> + && strncmp (SYMBOL_NATURAL_NAME(stack_chk_guard), "__stack_chk_guard",
> + strlen ("__stack_chk_guard")))
Why are you using the "natural" name rather than the linkage name?
Also, what would happen if the symbol name was __stack_chk_guard_1?
I presume that you'll still fail the instruction match later one,
but isn't a simple strcmp more accurate?
> + if (is_thumb)
> + {
> + unsigned int destreg;
> + unsigned short insn
> + = read_memory_unsigned_integer (pc + offset, 2, byte_order_for_code);
> + /* Step 2: ldr Rd, [Rn, #immed], encoding T1. */
Empty line after variable decls.
> + else
> + {
> + unsigned int destreg;
> + unsigned int insn
> + = read_memory_unsigned_integer (pc + offset, 4, byte_order_for_code);
> + /* Step 2: ldr Rd, [Rn, #immed], encoding A1. */
Same here.
> + if (post_prologue_pc)
> + {
> + post_prologue_pc
> + = arm_skip_stack_protector (post_prologue_pc, gdbarch);
> + }
We do not use the curly braces for single-instruction blocks:
if (post_prologue_pc)
post_prologue_pc
= arm_skip_stack_protector (post_prologue_pc, gdbarch);
--
Joel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch, arm] Skip insns generated by -fstack-protector
2010-12-23 4:16 ` Joel Brobecker
@ 2010-12-23 9:29 ` Yao Qi
2010-12-23 18:29 ` Joel Brobecker
0 siblings, 1 reply; 6+ messages in thread
From: Yao Qi @ 2010-12-23 9:29 UTC (permalink / raw)
To: Joel Brobecker; +Cc: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 5974 bytes --]
On 12/23/2010 12:02 PM, Joel Brobecker wrote:
>
>> +/* Extract the immediate from instruction movw/movt of encoding T. INSN1 is the
>> + first 16-bit of instruction, and INSN2 is the second 16-bit of
>> + instruction. */
>
> The first line is a little too long. It's exactly 80 characters, but
> I would prefer if it didn't exceed 78. I occasionally break that rule
> when the extra 2 characters allow the code formatting to be less ugly,
> but this is not the case here.
>
OK. Done.
>> +/* Extract the immediate form instruction movw/movt of encoding A. INSN is
> ^^^^ spelling `from'
Fixed this typo.
>
> Just on a personal suggestion that you don't have to agree with, I find:
>
>> - unsigned int imm = ((bits (insn, 0, 3) << 12)
>> - | (bits (insn, 10, 10) << 11)
>> - | (bits (inst2, 12, 14) << 8)
>> - | bits (inst2, 0, 7));
>
> ... more easily readable than the two-line form you used in the macro.
>
Yeah, I agree. Done.
>>
>> +
>> +/* Try to analyze the the instructions starting from PC, which load symbol
> ^^^^^^^
Sorry for this. Fixed.
>> + else
>> + {
>> + unsigned int insn
>> + = read_memory_unsigned_integer (pc, 4, byte_order_for_code);
>> + if ((insn & 0x0e5f0000) == 0x041f0000) /* ldr Rd, #immed */
>
> Empty line after local variable declaration.
>
Fixed with other instances.
>> +/* Try to skip a sequence of instructions used for stack protector. If PC
>> + points to the first instruction of this sequence, return the address of first
>> + instruction after this sequence, otherwise, return original PC.
>> +
>> + On arm, this sequence of instructions is composed by mainly three steps,
> ^^ of
Fixed.
>> + Step 1: load symbol __stack_chk_guard,
>> + Step 2: load from address of __stack_chk_guard,
>> + Step 3: store it to somewhere else.
>> +
>> + Usually, instructions on step 2 and step 3 are the same on various ARM
>> + archtectures. On step 2, it is one instruction 'ldr Rx, [Rn, #0]', and
^^^^^^^^^^^^
Fixed another typo here.
>> + on step 3, it is also one instruction 'str Rx, [r7, #immd]'. However,
>> + instructions in step 1 varies from different ARM archs. On ARMv7, they are,
> ^^^^^^ vary (plural)
>
> I am not sure that "from different ARM archs" is correct English (arches,
> btw). Suggest "from arch to arch" (we know that we're dealing with ARM
> only in this file). Perhaps "from ARM arch to arch" is also good English
> but I'm not sure.
I mean "different ARM architectures" are ARMv7, ARMv5t, ARMv6, and so
forth. On wikipedia, they are called "ARM Architecture"
http://en.wikipedia.org/wiki/ARM_architecture
So, I s/archs/architectures here.
>
>> + Since ldr/str is a very popular instruction, we can't use them as
>> + 'fingerprint' or 'signature' of stack protector sequence. Here we choose
>> + sequence {movw/movt, ldr}/ldr/str plus symbol __stack_chk_guard, if not
>> + stripped, as 'fingerprint' of stack protector cdoe sequence. */
> ^^ the ^^ a ^^^^^ code
Fixed.
>
>> +static CORE_ADDR
>> +arm_skip_stack_protector(CORE_ADDR pc, struct gdbarch *gdbarch)
>> +{
>> + enum bfd_endian byte_order_for_code = gdbarch_byte_order_for_code (gdbarch);
> [...]
>> + int is_thumb = arm_pc_is_thumb (gdbarch, pc);
>> +
>> + /* Try to parse the instructions in Step 1. */
>> + CORE_ADDR addr = arm_analyze_load_stack_chk_guard (pc, gdbarch,
>> + &basereg, &offset);
>> + if (!addr)
>> + return pc;
>
> This is a situation where we set ourselves up for adding code between
> the two groups of local variable declarations. We've done that in
> the past... I think that compilers detect that, now, so it's not so
> bad.
>
> I suggest that you move the declaration of variable `addr' to the
> first group of declarations, and then assign it later. Or you'll
> also need to add an empty line after its declaration.
Fixed by putting declaration of variable `addr' to the firs group.
>
>> + stack_chk_guard = lookup_minimal_symbol_by_pc (addr);
>> + /* If name of symbol doesn't start with '__stack_chk_guard', this
>> + instruction sequence is not for stack protector. If symbol is
>> + removed, we conservatively think this sequence is for stack protector. */
>> + if (stack_chk_guard
>> + && strncmp (SYMBOL_NATURAL_NAME(stack_chk_guard), "__stack_chk_guard",
>> + strlen ("__stack_chk_guard")))
>
> Why are you using the "natural" name rather than the linkage name?
In short, we should use linkage name here. Since __stack_chk_guard is
added by GCC during compilation without any information on language, the
"natural" name is the same as linkage name in this case. Fixed.
> Also, what would happen if the symbol name was __stack_chk_guard_1?
> I presume that you'll still fail the instruction match later one,
> but isn't a simple strcmp more accurate?
Replaced strncmp with strcmp.
>
>> + if (is_thumb)
>> + {
>> + unsigned int destreg;
>> + unsigned short insn
>> + = read_memory_unsigned_integer (pc + offset, 2, byte_order_for_code);
>> + /* Step 2: ldr Rd, [Rn, #immed], encoding T1. */
>
> Empty line after variable decls.
>
Done.
>> + else
>> + {
>> + unsigned int destreg;
>> + unsigned int insn
>> + = read_memory_unsigned_integer (pc + offset, 4, byte_order_for_code);
>> + /* Step 2: ldr Rd, [Rn, #immed], encoding A1. */
>
> Same here.
>
Done.
>> + if (post_prologue_pc)
>> + {
>> + post_prologue_pc
>> + = arm_skip_stack_protector (post_prologue_pc, gdbarch);
>> + }
>
> We do not use the curly braces for single-instruction blocks:
>
> if (post_prologue_pc)
> post_prologue_pc
> = arm_skip_stack_protector (post_prologue_pc, gdbarch);
>
Fixed.
--
Yao (é½å°§)
[-- Attachment #2: arm_skip_stack_protector_1223.patch --]
[-- Type: text/x-patch, Size: 8306 bytes --]
gdb/
* arm-tdep.c (thumb_analyze_prologue): Move some code ...
(EXTRACT_MOVW_MOVT_IMM_T): ... here. New macro.
(EXTRACT_MOVW_MOVT_IMM_A): New macro.
(arm_analyze_load_stack_chk_guard): New.
(arm_skip_stack_protector): New.
(arm_skip_prologue): Adjust post_prologue_pc by arm_skip_stack_protector.
diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index 636c1de..0d2ce01 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -495,6 +495,21 @@ skip_prologue_function (CORE_ADDR pc)
#define BranchDest(addr,instr) \
((CORE_ADDR) (((long) (addr)) + 8 + (sbits (instr, 0, 23) << 2)))
+/* Extract the immediate from instruction movw/movt of encoding T. INSN1 is
+ the first 16-bit of instruction, and INSN2 is the second 16-bit of
+ instruction. */
+#define EXTRACT_MOVW_MOVT_IMM_T(insn1, insn2) \
+ ((bits ((insn1), 0, 3) << 12) \
+ | (bits ((insn1), 10, 10) << 11) \
+ | (bits ((insn2), 12, 14) << 8) \
+ | bits ((insn2), 0, 7))
+
+/* Extract the immediate from instruction movw/movt of encoding A. INSN is
+ the 32-bit instruction. */
+#define EXTRACT_MOVW_MOVT_IMM_A(insn) \
+ ((bits ((insn), 16, 19) << 12) \
+ | bits ((insn), 0, 11))
+
/* Decode immediate value; implements ThumbExpandImmediate pseudo-op. */
static unsigned int
@@ -1003,10 +1018,8 @@ thumb_analyze_prologue (struct gdbarch *gdbarch,
else if ((insn & 0xfbf0) == 0xf240) /* movw Rd, #const */
{
- unsigned int imm = ((bits (insn, 0, 3) << 12)
- | (bits (insn, 10, 10) << 11)
- | (bits (inst2, 12, 14) << 8)
- | bits (inst2, 0, 7));
+ unsigned int imm
+ = EXTRACT_MOVW_MOVT_IMM_T (insn, inst2);
regs[bits (inst2, 8, 11)] = pv_constant (imm);
}
@@ -1129,6 +1142,188 @@ thumb_analyze_prologue (struct gdbarch *gdbarch,
return unrecognized_pc;
}
+
+/* Try to analyze the instructions starting from PC, which load symbol
+ __stack_chk_guard. Return the address of instruction after loading this
+ symbol, set the dest register number to *BASEREG, and set the size of
+ instructions for loading symbol in OFFSET. Return 0 if instructions are
+ not recognized. */
+
+static CORE_ADDR
+arm_analyze_load_stack_chk_guard(CORE_ADDR pc, struct gdbarch *gdbarch,
+ unsigned int *destreg, int *offset)
+{
+ enum bfd_endian byte_order_for_code = gdbarch_byte_order_for_code (gdbarch);
+ int is_thumb = arm_pc_is_thumb (gdbarch, pc);
+ unsigned int low, high, address;
+
+ address = 0;
+ if (is_thumb)
+ {
+ unsigned short insn1
+ = read_memory_unsigned_integer (pc, 2, byte_order_for_code);
+
+ if ((insn1 & 0xf800) == 0x4800) /* ldr Rd, #immed */
+ {
+ *destreg = bits (insn1, 8, 10);
+ *offset = 2;
+ address = bits (insn1, 0, 7);
+ }
+ else if ((insn1 & 0xfbf0) == 0xf240) /* movw Rd, #const */
+ {
+ unsigned short insn2
+ = read_memory_unsigned_integer (pc + 2, 2, byte_order_for_code);
+
+ low = EXTRACT_MOVW_MOVT_IMM_T (insn1, insn2);
+
+ insn1
+ = read_memory_unsigned_integer (pc + 4, 2, byte_order_for_code);
+ insn2
+ = read_memory_unsigned_integer (pc + 6, 2, byte_order_for_code);
+
+ /* movt Rd, #const */
+ if ((insn1 & 0xfbc0) == 0xf2c0)
+ {
+ high = EXTRACT_MOVW_MOVT_IMM_T (insn1, insn2);
+ *destreg = bits (insn2, 8, 11);
+ *offset = 8;
+ address = (high << 16 | low);
+ }
+ }
+ }
+ else
+ {
+ unsigned int insn
+ = read_memory_unsigned_integer (pc, 4, byte_order_for_code);
+
+ if ((insn & 0x0e5f0000) == 0x041f0000) /* ldr Rd, #immed */
+ {
+ address = bits (insn, 0, 11);
+ *destreg = bits (insn, 12, 15);
+ *offset = 4;
+ }
+ else if ((insn & 0x0ff00000) == 0x03000000) /* movw Rd, #const */
+ {
+ low = EXTRACT_MOVW_MOVT_IMM_A (insn);
+
+ insn
+ = read_memory_unsigned_integer (pc + 4, 4, byte_order_for_code);
+
+ if ((insn & 0x0ff00000) == 0x03400000) /* movt Rd, #const */
+ high = EXTRACT_MOVW_MOVT_IMM_A (insn);
+
+ address = (high << 16 | low);
+ *destreg = bits (insn, 12, 15);
+ *offset = 8;
+ }
+ }
+
+ return address;
+}
+
+/* Try to skip a sequence of instructions used for stack protector. If PC
+ points to the first instruction of this sequence, return the address of first
+ instruction after this sequence, otherwise, return original PC.
+
+ On arm, this sequence of instructions is composed of mainly three steps,
+ Step 1: load symbol __stack_chk_guard,
+ Step 2: load from address of __stack_chk_guard,
+ Step 3: store it to somewhere else.
+
+ Usually, instructions on step 2 and step 3 are the same on various ARM
+ architectures. On step 2, it is one instruction 'ldr Rx, [Rn, #0]', and
+ on step 3, it is also one instruction 'str Rx, [r7, #immd]'. However,
+ instructions in step 1 vary from different ARM architectures. On ARMv7,
+ they are,
+
+ movw Rn, #:lower16:__stack_chk_guard
+ movt Rn, #:upper16:__stack_chk_guard
+
+ On ARMv5t, it is,
+
+ ldr Rn, .Label
+ ....
+ .Lable:
+ .word __stack_chk_guard
+
+ Since ldr/str is a very popular instruction, we can't use them as
+ 'fingerprint' or 'signature' of stack protector sequence. Here we choose
+ sequence {movw/movt, ldr}/ldr/str plus symbol __stack_chk_guard, if not
+ stripped, as the 'fingerprint' of a stack protector cdoe sequence. */
+
+static CORE_ADDR
+arm_skip_stack_protector(CORE_ADDR pc, struct gdbarch *gdbarch)
+{
+ enum bfd_endian byte_order_for_code = gdbarch_byte_order_for_code (gdbarch);
+ unsigned int address, basereg;
+ struct minimal_symbol *stack_chk_guard;
+ int offset;
+ int is_thumb = arm_pc_is_thumb (gdbarch, pc);
+ CORE_ADDR addr;
+
+ /* Try to parse the instructions in Step 1. */
+ addr = arm_analyze_load_stack_chk_guard (pc, gdbarch,
+ &basereg, &offset);
+ if (!addr)
+ return pc;
+
+ stack_chk_guard = lookup_minimal_symbol_by_pc (addr);
+ /* If name of symbol doesn't start with '__stack_chk_guard', this
+ instruction sequence is not for stack protector. If symbol is
+ removed, we conservatively think this sequence is for stack protector. */
+ if (stack_chk_guard
+ && strcmp (SYMBOL_LINKAGE_NAME(stack_chk_guard), "__stack_chk_guard"))
+ return pc;
+
+ if (is_thumb)
+ {
+ unsigned int destreg;
+ unsigned short insn
+ = read_memory_unsigned_integer (pc + offset, 2, byte_order_for_code);
+
+ /* Step 2: ldr Rd, [Rn, #immed], encoding T1. */
+ if ((insn & 0xf800) != 0x6800)
+ return pc;
+ if (bits (insn, 3, 5) != basereg)
+ return pc;
+ destreg = bits (insn, 0, 2);
+
+ insn = read_memory_unsigned_integer (pc + offset + 2, 2,
+ byte_order_for_code);
+ /* Step 3: str Rd, [Rn, #immed], encoding T1. */
+ if ((insn & 0xf800) != 0x6000)
+ return pc;
+ if (destreg != bits (insn, 0, 2))
+ return pc;
+ }
+ else
+ {
+ unsigned int destreg;
+ unsigned int insn
+ = read_memory_unsigned_integer (pc + offset, 4, byte_order_for_code);
+
+ /* Step 2: ldr Rd, [Rn, #immed], encoding A1. */
+ if ((insn & 0x0e500000) != 0x04100000)
+ return pc;
+ if (bits (insn, 16, 19) != basereg)
+ return pc;
+ destreg = bits (insn, 12, 15);
+ /* Step 3: str Rd, [Rn, #immed], encoding A1. */
+ insn = read_memory_unsigned_integer (pc + offset + 4,
+ 4, byte_order_for_code);
+ if ((insn & 0x0e500000) != 0x04000000)
+ return pc;
+ if (bits (insn, 12, 15) != destreg)
+ return pc;
+ }
+ /* The size of total two instructions ldr/str is 4 on Thumb-2, while 8
+ on arm. */
+ if (is_thumb)
+ return pc + offset + 4;
+ else
+ return pc + offset + 8;
+}
+
/* Advance the PC across any function entry prologue instructions to
reach some "real" code.
@@ -1162,6 +1357,11 @@ arm_skip_prologue (struct gdbarch *gdbarch, CORE_ADDR pc)
= skip_prologue_using_sal (gdbarch, func_addr);
struct symtab *s = find_pc_symtab (func_addr);
+ if (post_prologue_pc)
+ post_prologue_pc
+ = arm_skip_stack_protector (post_prologue_pc, gdbarch);
+
+
/* GCC always emits a line note before the prologue and another
one after, even if the two are at the same address or on the
same line. Take advantage of this so that we do not need to
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch, arm] Skip insns generated by -fstack-protector
2010-12-23 9:29 ` Yao Qi
@ 2010-12-23 18:29 ` Joel Brobecker
2010-12-24 8:21 ` Yao Qi
0 siblings, 1 reply; 6+ messages in thread
From: Joel Brobecker @ 2010-12-23 18:29 UTC (permalink / raw)
To: Yao Qi; +Cc: gdb-patches
> gdb/
> * arm-tdep.c (thumb_analyze_prologue): Move some code ...
> (EXTRACT_MOVW_MOVT_IMM_T): ... here. New macro.
> (EXTRACT_MOVW_MOVT_IMM_A): New macro.
> (arm_analyze_load_stack_chk_guard): New.
> (arm_skip_stack_protector): New.
> (arm_skip_prologue): Adjust post_prologue_pc by arm_skip_stack_protector.
OK to commit - just watch out that the last line of the ChangeLog
entry seems a little too long....
--
Joel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch, arm] Skip insns generated by -fstack-protector
2010-12-23 18:29 ` Joel Brobecker
@ 2010-12-24 8:21 ` Yao Qi
0 siblings, 0 replies; 6+ messages in thread
From: Yao Qi @ 2010-12-24 8:21 UTC (permalink / raw)
To: Joel Brobecker; +Cc: gdb-patches
On 12/23/2010 08:08 PM, Joel Brobecker wrote:
>> gdb/
>> * arm-tdep.c (thumb_analyze_prologue): Move some code ...
>> (EXTRACT_MOVW_MOVT_IMM_T): ... here. New macro.
>> (EXTRACT_MOVW_MOVT_IMM_A): New macro.
>> (arm_analyze_load_stack_chk_guard): New.
>> (arm_skip_stack_protector): New.
>> (arm_skip_prologue): Adjust post_prologue_pc by arm_skip_stack_protector.
>
> OK to commit - just watch out that the last line of the ChangeLog
> entry seems a little too long....
>
Committed, with putting "arm_skip_stack_protector." to a new line in
ChangeLog.
http://sourceware.org/ml/gdb-cvs/2010-12/msg00112.html
--
Yao Qi
CodeSourcery
yao@codesourcery.com
(650) 331-3385 x739
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-12-24 5:14 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-12-09 13:45 [patch, arm] Skip insns generated by -fstack-protector Yao Qi
2010-12-20 14:08 ` Yao Qi
2010-12-23 4:16 ` Joel Brobecker
2010-12-23 9:29 ` Yao Qi
2010-12-23 18:29 ` Joel Brobecker
2010-12-24 8:21 ` Yao Qi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox