diff mbox

[committed] PR 57589: Fix logic error in driver-rs6000.c

Message ID 20130611232535.GA16789@ibm-tiger.the-meissners.org
State New
Headers show

Commit Message

Michael Meissner June 11, 2013, 11:25 p.m. UTC 
I committed the following patch as obvious, after making sure it builds and has
no regressions.  I was looking at driver-rs6000.c today, and I noticed that it
was returning an address to an auto buffer for processing -mcpu=native.

2013-06-11  Michael Meissner  <meissner@linux.vnet.ibm.com>

	PR target/57589
	* config/rs6000/driver-rs6000.c (elf_platform): Make buffer static
	to allow returning address to AT_PLATFORM name.

Comments

Jakub Jelinek June 11, 2013, 11:34 p.m. UTC | #1 
On Tue, Jun 11, 2013 at 07:25:36PM -0400, Michael Meissner wrote:
> I committed the following patch as obvious, after making sure it builds and has
> no regressions.  I was looking at driver-rs6000.c today, and I noticed that it
> was returning an address to an auto buffer for processing -mcpu=native.

Does it?  If I understand it right, the string itself isn't read into buf
(after all, what would translate the pointers in buf to point into other
parts of buf), /proc/self/auxv just contains the pairs of AT_* and the
corresponding values, for AT_PLATFORM that value is a char * pointing
somewhere into stack memory.

> 2013-06-11  Michael Meissner  <meissner@linux.vnet.ibm.com>
> 
> 	PR target/57589
> 	* config/rs6000/driver-rs6000.c (elf_platform): Make buffer static
> 	to allow returning address to AT_PLATFORM name.
> 
> Index: gcc/config/rs6000/driver-rs6000.c
> ===================================================================
> --- gcc/config/rs6000/driver-rs6000.c	(revision 199965)
> +++ gcc/config/rs6000/driver-rs6000.c	(working copy)
> @@ -167,7 +167,7 @@ elf_platform (void)
>  
>    if (fd != -1)
>      {
> -      char buf[1024];
> +      static char buf[1024];
>        ElfW(auxv_t) *av;
>        ssize_t n;

	Jakub
Michael Meissner June 11, 2013, 11:43 p.m. UTC | #2 
On Wed, Jun 12, 2013 at 01:34:59AM +0200, Jakub Jelinek wrote:
> On Tue, Jun 11, 2013 at 07:25:36PM -0400, Michael Meissner wrote:
> > I committed the following patch as obvious, after making sure it builds and has
> > no regressions.  I was looking at driver-rs6000.c today, and I noticed that it
> > was returning an address to an auto buffer for processing -mcpu=native.
> 
> Does it?  If I understand it right, the string itself isn't read into buf
> (after all, what would translate the pointers in buf to point into other
> parts of buf), /proc/self/auxv just contains the pairs of AT_* and the
> corresponding values, for AT_PLATFORM that value is a char * pointing
> somewhere into stack memory.

Ah, you are correct.  I misread the code, thinking it was returning a string,
and not a pointer to the string in memory.

I'll remove it tomorrow, and put in a comment for the next person.  Sigh....
Alan Modra Sept. 17, 2013, 2:11 a.m. UTC | #3 
On Tue, Jun 11, 2013 at 07:43:05PM -0400, Michael Meissner wrote:
> Ah, you are correct.  I misread the code, thinking it was returning a string,
> and not a pointer to the string in memory.
http://gcc.gnu.org/ml/gcc-patches/2013-06/msg00640.html

Patch reverted.
diff mbox

Patch

Index: gcc/config/rs6000/driver-rs6000.c
===================================================================
--- gcc/config/rs6000/driver-rs6000.c	(revision 199965)
+++ gcc/config/rs6000/driver-rs6000.c	(working copy)
@@ -167,7 +167,7 @@  elf_platform (void)
 
   if (fd != -1)
     {
-      char buf[1024];
+      static char buf[1024];
       ElfW(auxv_t) *av;
       ssize_t n;