[Bug c/59850] New: Support sparse-style pointer address spaces (type attributes)

[Bug c/59850] New: Support sparse-style pointer address spaces (type attributes)

[hpa at zytor dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59850

            Bug ID: 59850
           Summary: Support sparse-style pointer address spaces (type
                    attributes)
           Product: gcc
           Version: unknown
            Status: UNCONFIRMED
          Severity: enhancement
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: hpa at zytor dot com

The sparse C compile checker has a set of extensions to the gcc type attribute
set:

__attribute__((noderef))

__attribute__((address_space(N)))

... where N is an arbitrary integer.

sparse will warn if pointers tagged with different address spaces are mixed,
and if a pointer tagged with "noderef" is dereferenced.

It would be highly useful if these protections could be extended to gcc itself
and thus get more ubiquitously run.  The main use case, of course, is the Linux
kernel, but almost any low-level application which has to deal with multiple
"manually maintained" address spaces is likely to benefit.

__attribute__((force))

... usually used as part of a cast, is used to suppress the warning.

[Bug c/59850] Support sparse-style pointer address spaces (type attributes)

[josh at joshtriplett dot org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59850

Josh Triplett <josh at joshtriplett dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |josh at joshtriplett dot org

--- Comment #1 from Josh Triplett <josh at joshtriplett dot org> ---
A few additional behavior notes:

Casting a pointer from one address space to another must produce a warning,
unless the type used for the cast includes __attribute__((force)).  (Typically,
a codebase will have safe ways to perform such conversions, wrapped up in
functions with appropriate parameter and return types, with a force'd cast in
them; for instance, the kernel's copy_to_user and copy_from_user functions.)

__attribute__((force)) can be used in the type of a function parameter as well,
in which case the caller can pass a pointer to a different address space
without complaint.

Dereferences of a noderef type inside a sizeof() or typeof() must not generate
warnings.  For instance:

__attribute__((address_space(1),noderef)) int *a;
typeof(*a) b; /* b has type "int" */
typeof(*a) *c; /* c has type "int *" */

[Bug c/59850] Support sparse-style pointer address spaces (type attributes)

[tromey at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59850

Tom Tromey <tromey at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |tromey at gcc dot gnu.org

--- Comment #3 from Tom Tromey <tromey at gcc dot gnu.org> ---
I noticed this behavior and was wondering if it is intended:

bapiya. cat /tmp/q.c
__attribute__((force)) int *p;
int q = *p;
bapiya. sparse -Wno-decl /tmp/q.c
/tmp/q.c:2:10: warning: dereference of noderef expression


If so it would be good to know all the rules for "force" --
I stumbled across this by accident but then was wondering
if there are other rules I don't know of.

[Bug c/59850] Support sparse-style pointer address spaces (type attributes)

[josh at joshtriplett dot org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59850

--- Comment #4 from Josh Triplett <josh at joshtriplett dot org> ---
(In reply to Tom Tromey from comment #3)
> I noticed this behavior and was wondering if it is intended:
> 
> bapiya. cat /tmp/q.c
> __attribute__((force)) int *p;
> int q = *p;
> bapiya. sparse -Wno-decl /tmp/q.c
> /tmp/q.c:2:10: warning: dereference of noderef expression
> 
> 
> If so it would be good to know all the rules for "force" --
> I stumbled across this by accident but then was wondering
> if there are other rules I don't know of.

What version of sparse did you try that with?  I can't seem to reproduce that
with the current version, nor with older versions.

[Bug c/59850] Support sparse-style pointer address spaces (type attributes)

[tromey at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59850

--- Comment #5 from Tom Tromey <tromey at gcc dot gnu.org> ---
(In reply to Josh Triplett from comment #4)

> What version of sparse did you try that with?  I can't seem to reproduce
> that with the current version, nor with older versions.

The one from Fedora 20:

bapiya. sparse --version
0.4.4
bapiya. rpm -q sparse
sparse-0.4.5.rc1-3.fc20.x86_64


FWIW I have an initial patch for the address_space and force
attributes.  noderef looks a little harder, as I think it will
need to walk over expressions after they are built in order to
handle the "&" case properly.

[Bug c/59850] Support sparse-style pointer address spaces (type attributes)

[tromey at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59850

--- Comment #6 from Tom Tromey <tromey at gcc dot gnu.org> ---
Null pointer constants are treated specially, which makes sense,
but only if they have type "void *" and are in address space 0.
That is, this works:

#define NULL ((__attribute__ ((address_space (0))) void *) 0)
__attribute__((address_space (1))) int *p = NULL;

But this gets a warning:

#define NULL ((__attribute__ ((address_space (1))) void *) 0)
__attribute__((address_space (0))) int *p = NULL;

And so does this:

#define NULL ((int *) 0)
__attribute__((address_space (1))) int *p = NULL;


I'm not sure whether that last one ought to be an error or not.

[Bug c/59850] Support sparse-style pointer address spaces (type attributes)

[josh at joshtriplett dot org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59850

--- Comment #7 from Josh Triplett <josh at joshtriplett dot org> ---
(In reply to Tom Tromey from comment #6)
> Null pointer constants are treated specially, which makes sense,
> but only if they have type "void *" and are in address space 0.

Otherwise, they're not a null pointer constant, so they're not treated
specially. :)

A null pointer constant must be in address space 0; that's for compatibility
with the standard definition of NULL, to avoid needing unique NULL constants
for each possible address space (USER_NULL, IOMEM_NULL).  I don't think it
makes sense to treat a 0 in address space 1 as a null pointer constant; safer
to give a warning for mixing address spaces.  The goal isn't to treat 0
magically as an address in all address spaces; it's specifically to treat NULL
as the null for all address spaces.

> That is, this works:
> 
> #define NULL ((__attribute__ ((address_space (0))) void *) 0)
> __attribute__((address_space (1))) int *p = NULL;
> 
> But this gets a warning:
> 
> #define NULL ((__attribute__ ((address_space (1))) void *) 0)
> __attribute__((address_space (0))) int *p = NULL;

I can't think of a legitimate reason to have a null pointer constant in a
non-zero address space; there's already a null pointer constant, NULL,
effectively in all address spaces, so why would you want to redefine it?

And on the contrary, I can think of a very good reason to *have* this warning:
suppose you wanted to define an INVALID_FOO_POINTER in the foo address space,
and you decided to use 0 as the invalid value.  You should get a warning if you
try to use INVALID_FOO_POINTER with a non-foo pointer type; it shouldn't
magically pass silently just because the chosen value is 0.

> And so does this:
> 
> #define NULL ((int *) 0)
> __attribute__((address_space (1))) int *p = NULL;
> 
> 
> I'm not sure whether that last one ought to be an error or not.

That isn't a null pointer constant, since it isn't (void *); it can't be
converted to any other pointer type without warning, and I don't think it's
unreasonable to say it can't be converted to any other address space without
warning either.

[Bug c/59850] Support sparse-style pointer address spaces (type attributes)

[hpa at zytor dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59850

--- Comment #8 from H. Peter Anvin <hpa at zytor dot com> ---
Arguably the *right* way to solve that would be to support __null for C as well
as for C++.

[Bug c/59850] Support sparse-style pointer address spaces (type attributes)

[tromey at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59850

--- Comment #9 from Tom Tromey <tromey at gcc dot gnu.org> ---
(In reply to Josh Triplett from comment #7)

> I can't think of a legitimate reason to have a null pointer constant in a
> non-zero address space; there's already a null pointer constant, NULL,
> effectively in all address spaces, so why would you want to redefine it?

> That isn't a null pointer constant, since it isn't (void *); it can't be
> converted to any other pointer type without warning, and I don't think it's
> unreasonable to say it can't be converted to any other address space without
> warning either.

Thanks.  While the one case did seem borderline to me, overall my
concern is really about trying to understand all the cases, so I can
document the feature nicely.

[Bug c/59850] Support sparse-style pointer address spaces (type attributes)

[tromey at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59850

--- Comment #10 from Tom Tromey <tromey at gcc dot gnu.org> ---
Relatedly, could you say what the option "-Wcast-to-as" provides
beyond the normal warnings about changing address spaces?
I wonder if this is something I should be pulling in as well.
"man sparse" doesn't really say much here.

[Bug c/59850] Support sparse-style pointer address spaces (type attributes)

[josh at joshtriplett dot org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59850

--- Comment #11 from Josh Triplett <josh at joshtriplett dot org> ---
(In reply to Tom Tromey from comment #10)
> Relatedly, could you say what the option "-Wcast-to-as" provides
> beyond the normal warnings about changing address spaces?
> I wonder if this is something I should be pulling in as well.
> "man sparse" doesn't really say much here.

/tmp$ cat test.c 
static void *p;
static __attribute__((address_space(1))) void *p2 = p;
static __attribute__((address_space(1))) void *p3 =
(__attribute__((address_space(1))) void *)p;
static __attribute__((address_space(1))) void *p4 =
(__attribute__((address_space(1),force)) void *)p;
/tmp$ ~/src/sparse/sparse -Waddress-space test.c 
test.c:2:53: warning: incorrect type in initializer (different address spaces)
test.c:2:53:    expected void <asn:1>*static [toplevel] p2
test.c:2:53:    got void *static [toplevel] p
/tmp$ ~/src/sparse/sparse -Waddress-space -Wcast-to-as test.c 
test.c:2:53: warning: incorrect type in initializer (different address spaces)
test.c:2:53:    expected void <asn:1>*static [toplevel] p2
test.c:2:53:    got void *static [toplevel] p
test.c:3:54: warning: cast adds address space to expression (<asn:1>)

Without -Wcast-to-as, you won't get a warning for unforced casts that add an
address space.

Personally, I'd actually suggest merging the two in GCC, and always issuing
both sets of warnings.  I'd also suggest including the warnings in GCC's -Wall,
given that you'll only see them if you explicitly use an address_space
attribute.

[Bug c/59850] Support sparse-style pointer address spaces (type attributes)

[josh at joshtriplett dot org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59850

--- Comment #12 from Josh Triplett <josh at joshtriplett dot org> ---
(In reply to Tom Tromey from comment #9)
> (In reply to Josh Triplett from comment #7)
> 
> > I can't think of a legitimate reason to have a null pointer constant in a
> > non-zero address space; there's already a null pointer constant, NULL,
> > effectively in all address spaces, so why would you want to redefine it?
> 
> > That isn't a null pointer constant, since it isn't (void *); it can't be
> > converted to any other pointer type without warning, and I don't think it's
> > unreasonable to say it can't be converted to any other address space without
> > warning either.
> 
> Thanks.  While the one case did seem borderline to me, overall my
> concern is really about trying to understand all the cases, so I can
> document the feature nicely.

For the documentation, I'd suggest making it very clear that the magic "works
in all addresss spaces" behavior applies only to NULL ((void *)0), not to any
arbitrary pointer with the value 0.

[Bug c/59850] Support sparse-style pointer address spaces (type attributes)

[josh at joshtriplett dot org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59850

--- Comment #13 from Josh Triplett <josh at joshtriplett dot org> ---
(In reply to H. Peter Anvin from comment #8)
> Arguably the *right* way to solve that would be to support __null for C as
> well as for C++.

__null or nullptr?

[Bug c/59850] Support sparse-style pointer address spaces (type attributes)

[tromey at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59850

--- Comment #14 from Tom Tromey <tromey at gcc dot gnu.org> ---
(In reply to Josh Triplett from comment #11)

> Without -Wcast-to-as, you won't get a warning for unforced casts that add an
> address space.

Thanks!

> Personally, I'd actually suggest merging the two in GCC, and always issuing
> both sets of warnings.  I'd also suggest including the warnings in GCC's
> -Wall, given that you'll only see them if you explicitly use an
> address_space attribute.

My current patch adds just -Waddress-space and enables this
warning by default -- similar to what was decided for -Wdesignated-init.
This seems like something that might be
discussed and changed during the patch submission though.

[Bug c/59850] Support sparse-style pointer address spaces (type attributes)

[josh at joshtriplett dot org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59850

--- Comment #15 from Josh Triplett <josh at joshtriplett dot org> ---
(In reply to Tom Tromey from comment #14)
> (In reply to Josh Triplett from comment #11)
> > Personally, I'd actually suggest merging the two in GCC, and always issuing
> > both sets of warnings.  I'd also suggest including the warnings in GCC's
> > -Wall, given that you'll only see them if you explicitly use an
> > address_space attribute.
> 
> My current patch adds just -Waddress-space and enables this
> warning by default -- similar to what was decided for -Wdesignated-init.
> This seems like something that might be
> discussed and changed during the patch submission though.

Given that it only applies if you use the attribute, enabling it by default
seems even better than putting it in -Wall, unless there's some fundamental
objection to that.

[Bug c/59850] Support sparse-style pointer address spaces (type attributes)

[hpa at zytor dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59850

--- Comment #16 from H. Peter Anvin <hpa at zytor dot com> ---
Josh: nullptr pollutes the C user namespace, so it's not an option.

[Bug c/59850] Support sparse-style pointer address spaces (type attributes)

[tromey at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59850

--- Comment #17 from Tom Tromey <tromey at gcc dot gnu.org> ---
It seems that "force" works on function parameters and casts
but not direct assignments:

bapiya. nl -ba /tmp/q.c
     1    #define A(N) __attribute__((address_space (N)))
     2    #define force __attribute__((force))
     3    
     4    force int *ok;
     5    A(1) int *nope;
     6    
     7    void g(force int *p)
     8    {
     9      ok = p;
    10    }
    11    
    12    void f(void)
    13    {
    14      g(nope);
    15      ok = nope;
    16    }
bapiya. ./sparse -Wno-decl -Waddress-space /tmp/q.c
/tmp/q.c:15:6: warning: incorrect type in assignment (different address spaces)
/tmp/q.c:15:6:    expected int *[addressable] [toplevel] [assigned] ok
/tmp/q.c:15:6:    got int <asn:1>*[addressable] [toplevel] nope


(This is using git master sparse from today)

[Bug c/59850] Support sparse-style pointer address spaces (type attributes)

[tromey at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59850

--- Comment #18 from Tom Tromey <tromey at gcc dot gnu.org> ---
(In reply to Tom Tromey from comment #17)
> It seems that "force" works on function parameters and casts
> but not direct assignments:

It's also an error in conditional expressions and in a "return".

I can implement this exactly but I'm curious whether it is intended.

[Bug c/59850] Support sparse-style pointer address spaces (type attributes)

[josh at joshtriplett dot org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59850

--- Comment #19 from Josh Triplett <josh at joshtriplett dot org> ---
(In reply to Tom Tromey from comment #18)
> (In reply to Tom Tromey from comment #17)
> > It seems that "force" works on function parameters and casts
> > but not direct assignments:
> 
> It's also an error in conditional expressions and in a "return".
> 
> I can implement this exactly but I'm curious whether it is intended.

I brought this exact case up on linux-sparse, and Christopher Li's (quite
reasonable) perspective was that it doesn't really make sense to put "force" on
a variable to begin with (as opposed to a function parameter).  Given that, I
think one of two behaviors would be reasonable: either prohibit force entirely
on non-parameter variable declarations, or allow it and treat it much like
parameters (ignore extended type differences on assignment).  I'm mildly
inclined towards the latter.

I don't, however, think it's sensible to reproduce sparse's behavior entirely
here, allowing it but not having it take effect.  Either prohibit it or give it
a sensible semantic, preferably the latter.

[Bug c/59850] Support sparse-style pointer address spaces (type attributes)

[tromey at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59850

--- Comment #20 from Tom Tromey <tromey at gcc dot gnu.org> ---
(In reply to Josh Triplett from comment #19)

> I brought this exact case up on linux-sparse, and Christopher Li's (quite
> reasonable) perspective was that it doesn't really make sense to put "force"
> on a variable to begin with (as opposed to a function parameter).  Given
> that, I think one of two behaviors would be reasonable: either prohibit
> force entirely on non-parameter variable declarations, or allow it and treat
> it much like parameters (ignore extended type differences on assignment). 
> I'm mildly inclined towards the latter.
> 
> I don't, however, think it's sensible to reproduce sparse's behavior
> entirely here, allowing it but not having it take effect.  Either prohibit
> it or give it a sensible semantic, preferably the latter.

Allowing it is definitely simpler to implement.
Disallowing it for ordinary declarations would need some research
on my part.
BTW if you want to try it out I have a branch:
https://github.com/tromey/gcc/tree/add-sparse-attributes

[Bug c/59850] Support sparse-style pointer address spaces (type attributes)

[tromey at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=59850

--- Comment #21 from Tom Tromey <tromey at gcc dot gnu.org> ---
(In reply to Tom Tromey from comment #20)

> BTW if you want to try it out I have a branch:
> https://github.com/tromey/gcc/tree/add-sparse-attributes

This still needs a bit of work.
I rebased it to be more patchlike and wrote some ChangeLog
entries, but I still need to verify that all the behavior
is correct (I forgot a lot of state...) and also write the
documentation.

That said, recently I've been wondering whether this could
be better done as a plugin.  I started down the road of
simply patching gcc due, I think, to the designated_init
attribute, which can't really be done as a plugin.
But it seems that perhaps noderef/force/address_space could
be; and perhaps also bitwise and nocast.

In the "pro" column, as a plugin it could be maintained elsewhere.
That might be interesting.

In the "con" column, it's a pain if multiple projects want to
use these checks.  Then it's just one more thing to fetch.

I'm curious to hear your thoughts on the subject.

[Bug c/59850] Support sparse-style pointer address spaces (type attributes)

[manu at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=59850

Manuel López-Ibáñez <manu at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |manu at gcc dot gnu.org

--- Comment #22 from Manuel López-Ibáñez <manu at gcc dot gnu.org> ---
(In reply to Tom Tromey from comment #21)
> In the "pro" column, as a plugin it could be maintained elsewhere.
> That might be interesting.
> 
> In the "con" column, it's a pain if multiple projects want to
> use these checks.  Then it's just one more thing to fetch.

* We could add plugins to the GCC repository for things that are considered
generally useful but we don't want to bloat standard gcc. I am sure the FSF
will be happier if plugins live in the GCC repository and they are assigned to
them than if not.

* A plugin living in the GCC repository will likely have a lower barrier for
acceptance than code added to GCC.
>From gcc-bugs-return-455130-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org Fri Jun 27 11:29:22 2014
Return-Path: <gcc-bugs-return-455130-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org>
Delivered-To: listarch-gcc-bugs@gcc.gnu.org
Received: (qmail 12532 invoked by alias); 27 Jun 2014 11:29:22 -0000
Mailing-List: contact gcc-bugs-help@gcc.gnu.org; run by ezmlm
Precedence: bulk
List-Id: <gcc-bugs.gcc.gnu.org>
List-Archive: <http://gcc.gnu.org/ml/gcc-bugs/>
List-Post: <mailto:gcc-bugs@gcc.gnu.org>
List-Help: <mailto:gcc-bugs-help@gcc.gnu.org>
Sender: gcc-bugs-owner@gcc.gnu.org
Delivered-To: mailing list gcc-bugs@gcc.gnu.org
Received: (qmail 12491 invoked by uid 48); 27 Jun 2014 11:29:19 -0000
From: "pageexec at gmail dot com" <gcc-bugzilla@gcc.gnu.org>
To: gcc-bugs@gcc.gnu.org
Subject: [Bug c/59850] Support sparse-style pointer address spaces (type attributes)
Date: Fri, 27 Jun 2014 11:29:00 -0000
X-Bugzilla-Reason: CC
X-Bugzilla-Type: changed
X-Bugzilla-Watch-Reason: None
X-Bugzilla-Product: gcc
X-Bugzilla-Component: c
X-Bugzilla-Version: unknown
X-Bugzilla-Keywords:
X-Bugzilla-Severity: enhancement
X-Bugzilla-Who: pageexec at gmail dot com
X-Bugzilla-Status: UNCONFIRMED
X-Bugzilla-Priority: P3
X-Bugzilla-Assigned-To: unassigned at gcc dot gnu.org
X-Bugzilla-Target-Milestone: ---
X-Bugzilla-Flags:
X-Bugzilla-Changed-Fields:
Message-ID: <bug-59850-4-GNUT3mju7s@http.gcc.gnu.org/bugzilla/>
In-Reply-To: <bug-59850-4@http.gcc.gnu.org/bugzilla/>
References: <bug-59850-4@http.gcc.gnu.org/bugzilla/>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
X-Bugzilla-URL: http://gcc.gnu.org/bugzilla/
Auto-Submitted: auto-generated
MIME-Version: 1.0
X-SW-Source: 2014-06/txt/msg02212.txt.bz2
Content-length: 2132

https://gcc.gnu.org/bugzilla/show_bug.cgi?idY850

--- Comment #23 from PaX Team <pageexec at gmail dot com> ---
some data points based on my experience with the 'checker' gcc plugin in PaX:

1. the C address space infrastructure available since gcc 4.6 can be sort of
coerced into implementing the __user/__kernel/etc address spaces and it works
reasonably well (i'd say even better than sparse as it produces no false
positives in my experience and caught real bugs such as CVE-2014-0038).

2. __force itself presents a problem as its semantics isn't well defined and
only sparse knows how to model it. in gcc it cannot be an attribute as
attributes apply to the outermost variable/etc, e.g., you can't use them on a
pointee in a pointer context. what i did instead is that i introduced new
address spaces (__force_user/__force_kernel so far, __rcu/__iomem/etc will need
more of these) that replace the '__force something' combination with
__force_something (yes, this needs patching on the kernel side, and i haven't
done a thorough job of it but it works on my smaller configs at least). this
way the hijacked targetm.addr_space.legitimate_address_p callback can be taught
to allow/disallow the intended conversions.

3. designated_init is a tricky problem because by the time a plugin can examine
variable initializers, gcc will have lost the information. however with a trick
such unwanted initializers can instead be turned into a compile error (that
existing gcc infrastructure can detect). you can find it in spender's
randomize_layout plugin that's distributed in grsecurity.

4. as for maintaining a plugin for kernel and/or other use: inside the kernel
it'll need some kbuild infrastructure (there's one in PaX already, though it's
probably not 100% complete) and it's worked fine for our users for the past 3+
years now. for more  general use distros can package up plugins as they'd do
with any library (as plugins are really nothing more than that). note also that
keeping a plugin in the kernel tree will raise license problems (gplv2 vs
gplv3) but i guess the kernel list is the better forum for discussing that.

[Bug c/59850] Support sparse-style pointer address spaces (type attributes)

[manu at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=59850

--- Comment #24 from Manuel López-Ibáñez <manu at gcc dot gnu.org> ---
(In reply to PaX Team from comment #23)
> 3. designated_init is a tricky problem because by the time a plugin can
> examine variable initializers, gcc will have lost the information. however
> with a trick such unwanted initializers can instead be turned into a compile
> error (that existing gcc infrastructure can detect). you can find it in
> spender's randomize_layout plugin that's distributed in grsecurity.

There is a patch for GCC that was basically approved in January:
https://gcc.gnu.org/ml/gcc-patches/2014-01/msg01284.html

I am not sure why it hasn't been committed yet.
>From gcc-bugs-return-455142-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org Fri Jun 27 14:30:43 2014
Return-Path: <gcc-bugs-return-455142-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org>
Delivered-To: listarch-gcc-bugs@gcc.gnu.org
Received: (qmail 31277 invoked by alias); 27 Jun 2014 14:30:43 -0000
Mailing-List: contact gcc-bugs-help@gcc.gnu.org; run by ezmlm
Precedence: bulk
List-Id: <gcc-bugs.gcc.gnu.org>
List-Archive: <http://gcc.gnu.org/ml/gcc-bugs/>
List-Post: <mailto:gcc-bugs@gcc.gnu.org>
List-Help: <mailto:gcc-bugs-help@gcc.gnu.org>
Sender: gcc-bugs-owner@gcc.gnu.org
Delivered-To: mailing list gcc-bugs@gcc.gnu.org
Received: (qmail 31235 invoked by uid 48); 27 Jun 2014 14:30:39 -0000
From: "dimhen at gmail dot com" <gcc-bugzilla@gcc.gnu.org>
To: gcc-bugs@gcc.gnu.org
Subject: [Bug bootstrap/61631] New: [4.10 regression] ICE unwind-dw2.c:1639:5: internal compiler error: Segmentation fault
Date: Fri, 27 Jun 2014 14:30:00 -0000
X-Bugzilla-Reason: CC
X-Bugzilla-Type: new
X-Bugzilla-Watch-Reason: None
X-Bugzilla-Product: gcc
X-Bugzilla-Component: bootstrap
X-Bugzilla-Version: 4.10.0
X-Bugzilla-Keywords:
X-Bugzilla-Severity: normal
X-Bugzilla-Who: dimhen at gmail dot com
X-Bugzilla-Status: UNCONFIRMED
X-Bugzilla-Priority: P3
X-Bugzilla-Assigned-To: unassigned at gcc dot gnu.org
X-Bugzilla-Target-Milestone: ---
X-Bugzilla-Flags:
X-Bugzilla-Changed-Fields: bug_id short_desc product version bug_status bug_severity priority component assigned_to reporter
Message-ID: <bug-61631-4@http.gcc.gnu.org/bugzilla/>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
X-Bugzilla-URL: http://gcc.gnu.org/bugzilla/
Auto-Submitted: auto-generated
MIME-Version: 1.0
X-SW-Source: 2014-06/txt/msg02224.txt.bz2
Content-length: 4791

https://gcc.gnu.org/bugzilla/show_bug.cgi?ida631

            Bug ID: 61631
           Summary: [4.10 regression] ICE unwind-dw2.c:1639:5: internal
                    compiler error: Segmentation fault
           Product: gcc
           Version: 4.10.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: bootstrap
          Assignee: unassigned at gcc dot gnu.org
          Reporter: dimhen at gmail dot com

r212073 FAIL
r211865 PASS

Fedora 20, x86_64

configure --enable-checking=fold --disable-multilib
make
[stage1 pass]
[...]
/home/dimhen/build/gcc_current/./gcc/xgcc
-B/home/dimhen/build/gcc_current/./gcc/
-B/usr/local/x86_64-unknown-linux-gnu/bin/
-B/usr/local/x86_64-unknown-linux-gnu/lib/ -isystem
/usr/local/x86_64-unknown-linux-gnu/include -isystem
/usr/local/x86_64-unknown-linux-gnu/sys-include    -g -O2 -O2  -g -O2 -DIN_GCC
  -W -Wall -Wno-narrowing -Wwrite-strings -Wcast-qual -Wno-format
-Wstrict-prototypes -Wmissing-prototypes -Wold-style-definition  -isystem
./include   -fpic -mlong-double-80 -g -DIN_LIBGCC2 -fbuilding-libgcc
-fno-stack-protector   -fpic -mlong-double-80 -I. -I. -I../.././gcc
-I/home/dimhen/src/gcc_current/libgcc -I/home/dimhen/src/gcc_current/libgcc/.
-I/home/dimhen/src/gcc_current/libgcc/../gcc
-I/home/dimhen/src/gcc_current/libgcc/../include
-I/home/dimhen/src/gcc_current/libgcc/config/libbid -DENABLE_DECIMAL_BID_FORMAT
-DHAVE_CC_TLS  -DUSE_TLS -o unwind-dw2.o -MT unwind-dw2.o -MD -MP -MF
unwind-dw2.dep -fexceptions -c /home/dimhen/src/gcc_current/libgcc/unwind-dw2.c
-fvisibility=hidden -DHIDE_EXPORTS
/home/dimhen/src/gcc_current/libgcc/unwind-dw2.c: In function
'uw_install_context_1':
/home/dimhen/src/gcc_current/libgcc/unwind-dw2.c:1639:5: internal compiler
error: Segmentation fault
     _Unwind_SetSpColumn (target, target->cfa, &sp_slot);
     ^
0xae996b crash_signal
    /home/dimhen/src/gcc_current/gcc/toplev.c:337
0x86e62f fold_checksum_tree
    /home/dimhen/src/gcc_current/gcc/fold-const.c:14759
0x86ec3e fold_checksum_tree
    /home/dimhen/src/gcc_current/gcc/fold-const.c:14861
0x86ed58 fold_checksum_tree
    /home/dimhen/src/gcc_current/gcc/fold-const.c:14872
0x86e832 fold_checksum_tree
    /home/dimhen/src/gcc_current/gcc/fold-const.c:14791
0x86e832 fold_checksum_tree
    /home/dimhen/src/gcc_current/gcc/fold-const.c:14791
0x86e400 fold(tree_node*)
    /home/dimhen/src/gcc_current/gcc/fold-const.c:14706
0x66bab7 c_fully_fold_internal
    /home/dimhen/src/gcc_current/gcc/c-family/c-common.c:1365
0x66af75 c_fully_fold(tree_node*, bool, bool*)
    /home/dimhen/src/gcc_current/gcc/c-family/c-common.c:1097
0x62bcdf convert_arguments
    /home/dimhen/src/gcc_current/gcc/c/c-typeck.c:3112
0x62b569 build_function_call_vec(unsigned int, vec<unsigned int, va_heap,
vl_ptr>, tree_node*, vec<tree_node*, va_gc, vl_embed>*, vec<tree_node*, va_gc,
vl_embed>*)
    /home/dimhen/src/gcc_current/gcc/c/c-typeck.c:2903
0x62b9e2 c_build_function_call_vec(unsigned int, vec<unsigned int, va_heap,
vl_ptr>, tree_node*, vec<tree_node*, va_gc, vl_embed>*, vec<tree_node*, va_gc,
vl_embed>*)
    /home/dimhen/src/gcc_current/gcc/c/c-typeck.c:2988
0x655b4b c_parser_postfix_expression_after_primary
    /home/dimhen/src/gcc_current/gcc/c/c-parser.c:7729
0x655598 c_parser_postfix_expression
    /home/dimhen/src/gcc_current/gcc/c/c-parser.c:7563
0x652814 c_parser_unary_expression
    /home/dimhen/src/gcc_current/gcc/c/c-parser.c:6502
0x651d88 c_parser_cast_expression
    /home/dimhen/src/gcc_current/gcc/c/c-parser.c:6340
0x650b7d c_parser_binary_expression
    /home/dimhen/src/gcc_current/gcc/c/c-parser.c:6155
0x6505c2 c_parser_conditional_expression
    /home/dimhen/src/gcc_current/gcc/c/c-parser.c:5931
0x65034a c_parser_expr_no_commas
    /home/dimhen/src/gcc_current/gcc/c/c-parser.c:5849
0x65612e c_parser_expression
    /home/dimhen/src/gcc_current/gcc/c/c-parser.c:7856
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See <http://gcc.gnu.org/bugs.html> for instructions.
make[3]: *** [unwind-dw2.o] Error 1
make[3]: Leaving directory
`/home/dimhen/build/gcc_current/x86_64-unknown-linux-gnu/libgcc'
make[2]: *** [all-stage1-target-libgcc] Error 2
make[2]: Leaving directory `/home/dimhen/build/gcc_current'
make[1]: *** [stage1-bubble] Error 2
make[1]: Leaving directory `/home/dimhen/build/gcc_current'
make: *** [all] Error 2


$ /home/dimhen/build/gcc_current/./gcc/xgcc -v
Using built-in specs.
COLLECT_GCC=/home/dimhen/build/gcc_current/./gcc/xgcc
Target: x86_64-unknown-linux-gnu
Configured with: /home/dimhen/src/gcc_current/configure --enable-checking=fold
--disable-multilib
Thread model: posix
gcc version 4.10.0 20140627 (experimental) [trunk revision 212073] (GCC)

[Bug c/59850] Support sparse-style pointer address spaces (type attributes)

[tromey at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=59850

--- Comment #25 from Tom Tromey <tromey at gcc dot gnu.org> ---
(In reply to Manuel López-Ibáñez from comment #24)

> There is a patch for GCC that was basically approved in January:
> https://gcc.gnu.org/ml/gcc-patches/2014-01/msg01284.html
> 
> I am not sure why it hasn't been committed yet.

I'm going to submit the fixed up version this week.
>From gcc-bugs-return-455245-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org Sun Jun 29 02:50:58 2014
Return-Path: <gcc-bugs-return-455245-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org>
Delivered-To: listarch-gcc-bugs@gcc.gnu.org
Received: (qmail 2146 invoked by alias); 29 Jun 2014 02:50:56 -0000
Mailing-List: contact gcc-bugs-help@gcc.gnu.org; run by ezmlm
Precedence: bulk
List-Id: <gcc-bugs.gcc.gnu.org>
List-Archive: <http://gcc.gnu.org/ml/gcc-bugs/>
List-Post: <mailto:gcc-bugs@gcc.gnu.org>
List-Help: <mailto:gcc-bugs-help@gcc.gnu.org>
Sender: gcc-bugs-owner@gcc.gnu.org
Delivered-To: mailing list gcc-bugs@gcc.gnu.org
Received: (qmail 2077 invoked by uid 55); 29 Jun 2014 02:50:43 -0000
From: "jvdelisle at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org>
To: gcc-bugs@gcc.gnu.org
Subject: [Bug libfortran/61640] KIND=4 Character Array Internal Unit Read Fail
Date: Sun, 29 Jun 2014 02:50:00 -0000
X-Bugzilla-Reason: CC
X-Bugzilla-Type: changed
X-Bugzilla-Watch-Reason: None
X-Bugzilla-Product: gcc
X-Bugzilla-Component: libfortran
X-Bugzilla-Version: 4.10.0
X-Bugzilla-Keywords:
X-Bugzilla-Severity: normal
X-Bugzilla-Who: jvdelisle at gcc dot gnu.org
X-Bugzilla-Status: NEW
X-Bugzilla-Priority: P3
X-Bugzilla-Assigned-To: jvdelisle at gcc dot gnu.org
X-Bugzilla-Target-Milestone: ---
X-Bugzilla-Flags:
X-Bugzilla-Changed-Fields:
Message-ID: <bug-61640-4-mce1aUkHP3@http.gcc.gnu.org/bugzilla/>
In-Reply-To: <bug-61640-4@http.gcc.gnu.org/bugzilla/>
References: <bug-61640-4@http.gcc.gnu.org/bugzilla/>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
X-Bugzilla-URL: http://gcc.gnu.org/bugzilla/
Auto-Submitted: auto-generated
MIME-Version: 1.0
X-SW-Source: 2014-06/txt/msg02327.txt.bz2
Content-length: 684

https://gcc.gnu.org/bugzilla/show_bug.cgi?ida640

--- Comment #5 from Jerry DeLisle <jvdelisle at gcc dot gnu.org> ---
Author: jvdelisle
Date: Sun Jun 29 02:49:45 2014
New Revision: 212118

URL: https://gcc.gnu.org/viewcvs?rev!2118&root=gcc&view=rev
Log:
2014-06-28  Jerry DeLisle  <jvdelisle@gcc.gnu.org>

    PR libgfortran/61640
    * io/list_read.c (next_char_internal): Adjust the read length to
    a single wide character. (eat_spaces): Add missing paren.
    * io/unix.c (mem_read4): Use the correct mem_alloc function for
    wide character internal reads.

Modified:
    trunk/libgfortran/ChangeLog
    trunk/libgfortran/io/list_read.c
    trunk/libgfortran/io/unix.c

[Bug c/59850] Support sparse-style pointer address spaces (type attributes)

[tromey at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=59850

--- Comment #26 from Tom Tromey <tromey at gcc dot gnu.org> ---
(In reply to PaX Team from comment #23)
> some data points based on my experience with the 'checker' gcc plugin in PaX:

Hi.  Thanks for your reply.

I didn't easily find a git repository holding the checker plugin source.
Is it available somewhere?

> 1. the C address space infrastructure available since gcc 4.6 can be sort of
> coerced into implementing the __user/__kernel/etc address spaces and it
> works reasonably well (i'd say even better than sparse as it produces no
> false positives in my experience and caught real bugs such as CVE-2014-0038).

FWIW I looked into the existing C address space stuff in gcc and after
some deliberation decided not to use it.  It wasn't directly applicable
and tricks like taking some subset of the address space values for use
by the attribute would have meant difficult-to-test patches to various
back ends.

Your code apparently hijacks the target hook, which seems pretty clever,
though I guess more suitable in a plugin than in gcc proper.

> 2. __force itself presents a problem as its semantics isn't well defined and
> only sparse knows how to model it. in gcc it cannot be an attribute as
> attributes apply to the outermost variable/etc, e.g., you can't use them on
> a pointee in a pointer context.

Could you elaborate on this?
I think I looked at all the sparse test cases here and I don't recall
encountering any real issues (for address space I had to have a hack to
deal with function return types, but this didn't seem to affect force).
If you have extra tests not in sparse, that would be super.

[Bug c/59850] Support sparse-style pointer address spaces (type attributes)

[josh at joshtriplett dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=59850

--- Comment #27 from Josh Triplett <josh at joshtriplett dot org> ---
(In reply to Tom Tromey from comment #21)
> (In reply to Tom Tromey from comment #20)
> 
> > BTW if you want to try it out I have a branch:
> > https://github.com/tromey/gcc/tree/add-sparse-attributes
> 
> This still needs a bit of work.
> I rebased it to be more patchlike and wrote some ChangeLog
> entries, but I still need to verify that all the behavior
> is correct (I forgot a lot of state...) and also write the
> documentation.
> 
> That said, recently I've been wondering whether this could
> be better done as a plugin.  I started down the road of
> simply patching gcc due, I think, to the designated_init
> attribute, which can't really be done as a plugin.
> But it seems that perhaps noderef/force/address_space could
> be; and perhaps also bitwise and nocast.
> 
> In the "pro" column, as a plugin it could be maintained elsewhere.
> That might be interesting.
> 
> In the "con" column, it's a pain if multiple projects want to
> use these checks.  Then it's just one more thing to fetch.
> 
> I'm curious to hear your thoughts on the subject.

Given the goal of providing compatibility with the existing support in Sparse
(and potentially surpassing it in the future with the benefit of better
analysis and compiler infrastructure backing it up), I strongly feel that this
needs to exist in the default distribution of GCC, invokable without any
barriers beyond an additional warning flag.  (And usually not even that;
anything that only emits warnings on code using an extension attribute should
get turned on by default.)

A plugin shipped with GCC and enabled by default could potentially provide that
benefit as well, but that goes against the only "pro" you listed.  I don't
think maintaining this elsewhere makes sense.  Meanwhile, the "con" you listed
seems far more serious.  I'd like to see all projects currently using Sparse
able to transparently take advantage of this.

Please let me know what I can do to help complete this branch.  I'd be happy to
help write the documentation, for instance.

[Bug c/59850] Support sparse-style pointer address spaces (type attributes)

[tromey at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=59850

--- Comment #28 from Tom Tromey <tromey at gcc dot gnu.org> ---
> Please let me know what I can do to help complete this branch.  I'd be happy
> to help write the documentation, for instance.

It's maybe not quite ready for testing.  But if you want to 
give it a try and see how it fares, that would help.
My spare time is pretty limited, so any sort of help is
appreciated ... docs, fixes, checking that I got the test cases
correct, taking over the branch entirely... :)

FWIW I re-sent the designated_init patch for review.
If you want to comment on that one that would also be great.

[Bug c/59850] Support sparse-style pointer address spaces (type attributes)

[tromey at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=59850

--- Comment #31 from Tom Tromey <tromey at gcc dot gnu.org> ---
(In reply to Tom Tromey from comment #28)
> > Please let me know what I can do to help complete this branch.  I'd be happy
> > to help write the documentation, for instance.
> 
> It's maybe not quite ready for testing. 

I did a bit more work on it now.
Josh, could you take a look?
It's git@github.com:tromey/gcc.git add-sparse-attributes
Most useful, I think, would be a review of the tests, to
make sure that they are complete and correct; and a review
of the docs.
force, adddress_space, and noderef are the final 5 commits on the branch.
I didn't squash it all into submission form yet.

[Bug c/59850] Support sparse-style pointer address spaces (type attributes)

[tromey at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=59850

--- Comment #32 from Tom Tromey <tromey at gcc dot gnu.org> ---
(In reply to Tom Tromey from comment #31)

> force, adddress_space, and noderef are the final 5 commits on the branch.

Err, 8 commits.

[Bug c/59850] Support sparse-style pointer address spaces (type attributes)

[tromey at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=59850

--- Comment #33 from Tom Tromey <tromey at gcc dot gnu.org> ---
My current patch fails on some of the sparse validation tests.
E.g., attr-in-parameter.c:

attr_in_parameter.c:8:4: warning: assignment from pointer to different address
space [-Waddress-space]

This happens due to this line:

static int (A *p);

I have yet to debug this one.


Also from noderef.c I found out that sparse accepts "noderef"
on non-pointer types:

    struct x __A x;

I hadn't realized this was possible.
It seems somewhat weird to me, but I suppose I'll update my patch
to follow.

This also affects address_space, as shown by typeof-attribute.c:

       unsigned int __percpu x;

[Bug c/59850] Support sparse-style pointer address spaces (type attributes)

[tromey at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=59850

--- Comment #34 from Tom Tromey <tromey at gcc dot gnu.org> ---
Created attachment 33277
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=33277&action=edit
initial patch

This is my current patch.  I'm uploading it for safekeeping
as I probably won't be working on this in the near future.

Note that as mentioned in the comments, this doesn't pass
the sparse validation suite.  So, it needs some work...
feel free to take it over if you are so moved.  A good
chunk of it should still be relevant even with the needed
updates.

[Bug c/59850] Support sparse-style pointer address spaces (type attributes)

[dmalcolm at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=59850

David Malcolm <dmalcolm at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at gcc dot gnu.org      |dmalcolm at gcc dot gnu.org
             Status|UNCONFIRMED                 |ASSIGNED
     Ever confirmed|0                           |1
                 CC|                            |dmalcolm at gcc dot gnu.org
   Last reconfirmed|                            |2022-09-26

--- Comment #35 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
Created attachment 53631
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=53631&action=edit
Refreshed version of Tom Tromey's patch

This is a refreshed version of Tom Tromey's patch from 2014, against last
week's trunk (specifically, against dc829c7613ddf562d1aecaf22eda965e87108ac8).

The new tests pass; am currently attempting a bootstrap and full regression
test.

Has the same caveats that Tom noted in the earlier comments.

[Bug c/59850] Support sparse-style pointer address spaces (type attributes)

[mpolacek at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=59850

Marek Polacek <mpolacek at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mpolacek at gcc dot gnu.org

--- Comment #36 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
(In reply to H. Peter Anvin from comment #16)
> Josh: nullptr pollutes the C user namespace, so it's not an option.

FWIW, C23 added the nullptr keyword which gcc already implements:
https://gcc.gnu.org/git/?p=gcc.git;a=commitdiff;h=60d84e82639e25b025a926b19ec5a92fba4447ce

[Bug c/59850] Support sparse-style pointer address spaces (type attributes)

[hpa at zytor dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=59850

--- Comment #37 from H. Peter Anvin <hpa at zytor dot com> ---
One would assume that there would be __foo__ aliases for the attribute names
like all the other ones.