All of lore.kernel.org
 help / color / mirror / Atom feed
From: Chen Gang <gang.chen.5i5j@gmail.com>
To: Jeff Epler <jepler@unpythonic.net>
Cc: cmetcalf@tilera.com,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] arch: tile: kernel: kgdb.c: Use memcpy() instead of pointer copy one by one
Date: Wed, 12 Nov 2014 23:43:08 +0800	[thread overview]
Message-ID: <5463800C.8050404@gmail.com> (raw)
In-Reply-To: <20141112132741.GA53560@unpythonic.net>

On 11/12/2014 09:27 PM, Jeff Epler wrote:
> It seems there's additional background required to understand the
> diagnostic:
> 
> asm/ptrace.h has
>     struct pt_regs {
>         /* tp, sp, and lr must immediately follow regs[] for aliasing. */
>         pt_reg_t regs[53];
>         pt_reg_t tp;            /* aliases regs[TREG_TP] */
>         pt_reg_t sp;            /* aliases regs[TREG_SP] */
>         pt_reg_t lr;            /* aliases regs[TREG_LR] */
> and the intended copy overwites all of regs[], plus tp, sp, and lr.
> 
> It's intended for thread_regs.regs[TREG_TP] to alias to thread_regs.tp,
> though in C this is undefined behavior (it dereferences a pointer past
> the end of the structure).

Yeah.

>>>   arch/tile/kernel/kgdb.c:140:31: warning: iteration 53u invokes undefined behavior [-Waggressive-loop-optimizations]
>>>      *(ptr++) = thread_regs->regs[reg];
> 
> If compilers are beginning to exploit the rule that indexing past the
> end of an array is UB, then the way that these register aliases are
> created may need to be revisited with careful attention to what the C
> standard actually says; I'm just going by memory.

"thread_regs.tp" and "TREG_SP" (not "TREG_TP") are all used, at present.
For me, both of them can let code readable and clearer, and they are not
conflict (variable let address clearer, macro let index clearer).

So for me, only "memcpy() instead of 'for' looping" is engough. Not
consider about compiler's warning, memcpy() can get a little higher
performance and simple code.

>                                                       (I assume the
> compiler could do things like replace an intended load from memory with
> a constant load or even no load at all)
> 

Excuse me, my English is not quite well, I can not understand what you
said above. (If necessary, please help provide more details for it).

Thanks.
-- 
Chen Gang

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

  reply	other threads:[~2014-11-12 15:43 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-11-01 13:17 [PATCH] arch: tile: kernel: kgdb.c: Use memcpy() instead of pointer copy one by one Chen Gang
2014-11-12  2:11 ` Chen Gang
2014-11-12 13:27   ` Jeff Epler
2014-11-12 15:43     ` Chen Gang [this message]
2014-11-12 19:29       ` Jeff Epler
2014-11-12 20:15   ` Chris Metcalf
  -- strict thread matches above, loose matches on Subject: below --
2014-11-13  0:08 Chen Gang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5463800C.8050404@gmail.com \
    --to=gang.chen.5i5j@gmail.com \
    --cc=cmetcalf@tilera.com \
    --cc=jepler@unpythonic.net \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.