2015-07-10 21:48:58

by Chen Gang

[permalink] [raw]
Subject: [PATCH v2] cris: arch-v10: kgdb: Add volatile for static variable is_dyn_brkp

Within one C file, current gcc can optimize the global static variables
according to the C code, but it will skip assembly code -- it will pass
them to gas directly.

if the static variable is used between C code and assembly code in one C
file (e.g. is_dyn_brkp in kgdb.c), it needs volatile to let gcc know it
should not be optimized, or it may cause issue.

The related error in this case:

LD init/built-in.o
arch/cris/arch-v10/kernel/built-in.o: In function `kgdb_handle_breakpoint':
(.text+0x2aca): undefined reference to `is_dyn_brkp'
arch/cris/arch-v10/kernel/built-in.o: In function `is_static':
kgdb.c:(.text+0x2ada): undefined reference to `is_dyn_brkp'


Signed-off-by: Chen Gang <[email protected]>
---
arch/cris/arch-v10/kernel/kgdb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/cris/arch-v10/kernel/kgdb.c b/arch/cris/arch-v10/kernel/kgdb.c
index 5b61335..8faddd3 100644
--- a/arch/cris/arch-v10/kernel/kgdb.c
+++ b/arch/cris/arch-v10/kernel/kgdb.c
@@ -351,7 +351,7 @@ char internal_stack[INTERNAL_STACK_SIZE];
breakpoint to be handled. A static breakpoint uses the content of register
BRP as it is whereas a dynamic breakpoint requires subtraction with 2
in order to execute the instruction. The first breakpoint is static. */
-static unsigned char is_dyn_brkp = 0;
+static volatile unsigned char is_dyn_brkp;

/********************************* String library ****************************/
/* Single-step over library functions creates trap loops. */
--
1.9.3


2015-08-04 15:01:29

by Hans-Peter Nilsson

[permalink] [raw]
Subject: Re: [PATCH v2] cris: arch-v10: kgdb: Add volatile for static variable is_dyn_brkp

> From: Chen Gang <[email protected]>
> Date: Fri, 10 Jul 2015 23:50:07 +0200

> Within one C file, current gcc can optimize the global static variables
> according to the C code, but it will skip assembly code -- it will pass
> them to gas directly.
>
> if the static variable is used between C code and assembly code in one C
> file (e.g. is_dyn_brkp in kgdb.c), it needs volatile to let gcc know it
> should not be optimized, or it may cause issue.

In this case it's *mostly* a matter of taste but please avoid
using volatile as a hammer when there are other tools available.

> -static unsigned char is_dyn_brkp = 0;
> +static volatile unsigned char is_dyn_brkp;

Please instead use "__used", i.e.
+static unsigned char __used is_dyn_brkp = 0;

brgds, H-P

2015-08-05 14:18:00

by Chen Gang

[permalink] [raw]
Subject: Re: [PATCH v2] cris: arch-v10: kgdb: Add volatile for static variable is_dyn_brkp

On 8/4/15 23:01, Hans-Peter Nilsson wrote:
>> From: Chen Gang <[email protected]>
>> Date: Fri, 10 Jul 2015 23:50:07 +0200
>
>> Within one C file, current gcc can optimize the global static variables
>> according to the C code, but it will skip assembly code -- it will pass
>> them to gas directly.
>>
>> if the static variable is used between C code and assembly code in one C
>> file (e.g. is_dyn_brkp in kgdb.c), it needs volatile to let gcc know it
>> should not be optimized, or it may cause issue.
>
> In this case it's *mostly* a matter of taste but please avoid
> using volatile as a hammer when there are other tools available.
>
>> -static unsigned char is_dyn_brkp = 0;
>> +static volatile unsigned char is_dyn_brkp;
>
> Please instead use "__used", i.e.
> +static unsigned char __used is_dyn_brkp = 0;
>

OK, thanks. I shall send patch v3 for it, next.

Thanks.
--
Chen Gang

Open, share, and attitude like air, water, and life which God blessed