Re: [google] Check if the nonnull attribute is applied to 'this' (issue4446070)
On 05/04/2011 05:08 AM, Richard Guenther wrote: On Tue, May 3, 2011 at 10:10 PM,jason.merr...@gmail.com wrote: As I commented on the -Wnonnull patch, rather than complain about people getting the argument number wrong we should ignore 'this' (and other artificial parms) for attribute argument numbers. I don't think we can reasonably change that now for existing attributes ... Ah, no, I suppose not. Jason
Re: [google] Check if the nonnull attribute is applied to 'this' (issue4446070)
On 04/29/2011 04:08 PM, Basile Starynkevitch wrote: [...] which is also what I expect. I have no idea if it is conforming to standards. But I notice that the test about this is remaining in the code. So apparently GCC 4.6 does not make the hypothesis that this is never null, otherwise it would have optimized the test by removing it. What I don't know (and I am asking) is if such an hypothetical optimization is conforming to standards. My biased feeling is that it is not (because I found no explicit unambiguous mention in standard legal text that this is never null). 9.3.1 seems pretty clear: If a non-static member function of a class X is called for an object that is not of type X, or of a type derived from X, the behavior is undefined. A null pointer does not point to an object of type X, so the optimization would be conforming. Jason
Re: [google] Check if the nonnull attribute is applied to 'this' (issue4446070)
On Fri, Apr 29, 2011 at 5:08 PM, Diego Novillo dnovi...@google.com wrote:
This patch from Le-Chun Wu adds support to check whether a nonnull
attribute is applied to 'this' pointer for non-static methods.
OK for trunk? Applied to google/main
2011-04-27 Le-Chun Wu l...@google.com
Google ref 45339.
* c-common.c (handle_nonnull_attribute): Check whether the nonnull
attribute is applied to the 'this' pointer for non-static methods.
testsuite/ChangeLog.google-main
2011-04-27 Le-Chun Wu l...@google.com
* g++.dg/warn/nonnull2.C: New.
diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index c6dc649..a1702f8 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -7434,7 +7434,7 @@ handle_nonnull_attribute (tree *node, tree ARG_UNUSED
(name),
/* Argument list specified. Verify that each argument number references
a pointer argument. */
- for (attr_arg_num = 1; args; args = TREE_CHAIN (args))
+ for (attr_arg_num = 1; args; args = TREE_CHAIN (args), attr_arg_num++)
{
tree argument;
unsigned HOST_WIDE_INT arg_num = 0, ck_num;
@@ -7466,6 +7466,7 @@ handle_nonnull_attribute (tree *node, tree ARG_UNUSED
(name),
return NULL_TREE;
}
+
spurious white-space change.
if (TREE_CODE (TREE_VALUE (argument)) != POINTER_TYPE)
{
error (nonnull argument references non-pointer operand
(argument %lu, operand %lu),
@@ -7473,6 +7474,11 @@ handle_nonnull_attribute (tree *node, tree ARG_UNUSED
(name),
*no_add_attrs = true;
return NULL_TREE;
}
+
+ if (TREE_CODE (type) == METHOD_TYPE arg_num == 1)
+ warning (OPT_Wattributes,
+ nonnull argument references 'this' pointer (argument
%lu, operand %lu),
+ (unsigned long) attr_arg_num, (unsigned long) arg_num);
}
}
diff --git a/gcc/testsuite/g++.dg/warn/nonnull2.C
b/gcc/testsuite/g++.dg/warn/nonnull2.C
new file mode 100644
index 000..03006b1
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/nonnull2.C
@@ -0,0 +1,14 @@
+// Test that nonnull attribute should not be applied to 'this' pointer.
+// { dg-do compile }
+
+#define NULL 0
+
+class Foo {
+ public:
+ void method1(const int *ptr) __attribute__((nonnull(1, 2))); // {
dg-warning nonnull argument references 'this' pointer }
+ void method2(int *ptr1, int a, int *ptr2) __attribute__((nonnull(2, 3,
4))); // { dg-error nonnull argument references non-pointer operand }
+ static void func3(int *ptr) __attribute__((nonnull(1))); // should not warn
+ Foo(char *str) __attribute__((nonnull())) {}
+};
+
+int func4(int *ptr1, int a) __attribute__((nonnull(1))); // should not warn
--
1.7.3.1
--
This patch is available for review at http://codereview.appspot.com/4446070
Re: [google] Check if the nonnull attribute is applied to 'this' (issue4446070)
On 2011/04/29 15:12:52, richard.guenther_gmail.com wrote: + spurious white-space change. Thanks. Fixed. Diego. http://codereview.appspot.com/4446070/
Re: [google] Check if the nonnull attribute is applied to 'this' (issue4446070)
On Fri, 29 Apr 2011 11:08:24 -0400 (EDT)
dnovi...@google.com (Diego Novillo) wrote:
This patch from Le-Chun Wu adds support to check whether a nonnull
attribute is applied to 'this' pointer for non-static methods.
This bring me to a question. Does the C++ standard imply that this is
never a null pointer? (Last time I looked into a C++ standard, I was
not able to give a firm answer).
In other words, with
class Myclass {
int x;
public:
Myclass(int z=0): x(z) {};
int get(void) const { if (this) return x; else return 0; };
~Myclass() { };
};
can a C++ standard conforming compiler optimize the get () member
function to
inline int Myclass::get(void) { return x; };
(I tend to believe that such an optimization is incorrect, but I am not
sure). can a C++ program call
Myclass *t = null_ptr;
return t-get();
(I might believe this is not conforming to standards)
I feel that testing that this is not null is occasionnally useful. For
instance, if I coded a trivial interpreter (e.g. of a Lua or Python or
Lisp-like language) I would be tempted, if that language has a nil
value, to implement values as pointers to objects and to represent nil
as a C++ null_ptr. So I might even want to code something like
// my abstract top superclass for values
class Value {
virtual ~Value() {};
Value () {};
virtual void do_print(void) const = 0;
public:
void print(void) const { if (this) this-do_print(); };
};
class IntegerValue : public Value {
int x;
public:
IntegerValue (int z) : Value(), x(z) {}:
virtual void do_print (void) { cout x; };
};
But I am not sure that such code is standard C++. However, GCC seems to
compile it as I expect! At least I still hope that future versions of
GCC would accept it (perhaps in some permissive mode, or with warnings)
Regards.
PS: By standard C++, I mean some recent or future C++ ISO standard (eg C++1x or
C++0x).
--
Basile STARYNKEVITCH http://starynkevitch.net/Basile/
email: basileatstarynkevitchdotnet mobile: +33 6 8501 2359
8, rue de la Faiencerie, 92340 Bourg La Reine, France
*** opinions {are only mine, sont seulement les miennes} ***
Re: [google] Check if the nonnull attribute is applied to 'this' (issue4446070)
On Fri, Apr 29, 2011 at 14:46, Basile Starynkevitch bas...@starynkevitch.net wrote: On Fri, 29 Apr 2011 11:08:24 -0400 (EDT) dnovi...@google.com (Diego Novillo) wrote: This patch from Le-Chun Wu adds support to check whether a nonnull attribute is applied to 'this' pointer for non-static methods. This bring me to a question. Does the C++ standard imply that this is never a null pointer? (Last time I looked into a C++ standard, I was not able to give a firm answer). I didn't find a specific section either, but that's my understanding. The this pointer can never be NULL. Jason, Lawrence? Diego.
Re: [google] Check if the nonnull attribute is applied to 'this' (issue4446070)
On Apr 29, 2011, at 11:46 AM, Basile Starynkevitch wrote:
On Fri, 29 Apr 2011 11:08:24 -0400 (EDT)
dnovi...@google.com (Diego Novillo) wrote:
This patch from Le-Chun Wu adds support to check whether a nonnull
attribute is applied to 'this' pointer for non-static methods.
This bring me to a question. Does the C++ standard imply that this is
never a null pointer?
Does this:
4 Certain other operations are described in this International Standard
as undefined (for example, the effect of dereferencing the null
pointer). [Note: this International Standard imposes no requirements
on the behavior of programs that contain undefined behavior. ]
answer your question? Note, this is exactly the same as C. You can't do it in
C either:
struct A {
int x;
} *pa = 0;
int main() {
pa-x = 1;
}
If you run it, it will crash on all good OSes. In C++, it will also crash.
Re: [google] Check if the nonnull attribute is applied to 'this' (issue4446070)
On Fri, 29 Apr 2011 12:32:24 -0700
Mike Stump mikest...@comcast.net wrote:
On Apr 29, 2011, at 11:46 AM, Basile Starynkevitch wrote:
This bring me to a question. Does the C++ standard imply that this is
never a null pointer?
Does this:
4 Certain other operations are described in this International Standard
as undefined (for example, the effect of dereferencing the null
pointer). [Note: this International Standard imposes no requirements
on the behavior of programs that contain undefined behavior. ]
answer your question?
Not really.
Note, this is exactly the same as C. You can't do it in C either:
struct A {
int x;
} *pa = 0;
int main() {
pa-x = 1;
}
If you run it, it will crash on all good OSes. In C++, it will also crash.
The examples I gave did not crash, because they are non-virtual member
functions. I gave as example
class Myclass {
int x;
public:
Myclass(int z=0): x(z) {};
int get(void) const { if (this) return x; else return 0; };
~Myclass() { };
};
Please notice that get above is a non-virtual member *function*
In the old days when C++ was a translator to C, the
generated C code would be something like
int Myclass__get (MyClass* this)
{ if (this) return this-x; else return 0; }
And such C code won't crash and is conforming to (old and current) C
standard[s].
My example did not dereference a null pointer! I did test some similar
code and on my Linux machine it did work as I expect.
So I still don't know if this can be null or not. After all, it is
(in C++ parlance) a pointer, not a reference, and C++ pointers can be
null.
with gcc version 4.6.1 20110421 (prerelease) (Debian 4.6.0-5)
(Debian/Experimental on AMD64) the following file
/// file nullthis.cc
class Myclass {
int x;
public:
Myclass(int z=0): x(z) {};
int get(void) const { if (this) return x; else return 0; };
~Myclass() { };
};
extern C int myget (Myclass *c) { return c-get(); };
eof nullthis.cc
is compiled with g++ -Wall -O3 -fverbose-asm -S nullthis.cc
without warnings, and the only produced function in nullthis.s is
.globl myget
.type myget, @function
myget:
.LFB7:
.cfi_startproc
xorl%eax, %eax # D.2112
testq %rdi, %rdi # c
je .L2 #,
movl(%rdi), %eax# MEM[(const struct Myclass
*)c_1(D)].x, D.2112 .L2:
rep
ret
.cfi_endproc
.LFE7:
.size myget, .-myget
.ident GCC: (Debian 4.6.0-5) 4.6.1 20110421
(prerelease)
This is exactly what I expect. But I don't know if it conforms to
standard.
When compiling with g++ -Wall -O3 -fdump-tree-all -fverbose-asm -S
nullthis.cc I have (among others) a dump file
% cat nullthis.cc.131t.phiopt3
;; Function int myget(Myclass*) (myget)
int myget(Myclass*) (struct Myclass * c)
{
int D.2112;
bb 2:
if (c_1(D) != 0B)
goto bb 3;
else
goto bb 4;
bb 3:
D.2112_4 = MEM[(const struct Myclass *)c_1(D)].x;
bb 4:
# D.2112_5 = PHI 0(2), D.2112_4(3)
return D.2112_5;
}
which is also what I expect. I have no idea if it is conforming to
standards. But I notice that the test about this is remaining in the
code. So apparently GCC 4.6 does not make the hypothesis that this is
never null, otherwise it would have optimized the test by removing it.
What I don't know (and I am asking) is if such an hypothetical
optimization is conforming to standards. My biased feeling is that it
is not (because I found no explicit unambigous mention in standard
legal text that this is never null).
Of course, calling a virtual member function on null is obviously an
undefined behavior (and SIGSEGV under Linux).
Regards.
--
Basile STARYNKEVITCH http://starynkevitch.net/Basile/
email: basileatstarynkevitchdotnet mobile: +33 6 8501 2359
8, rue de la Faiencerie, 92340 Bourg La Reine, France
*** opinions {are only mine, sont seulement les miennes} ***
Re: [google] Check if the nonnull attribute is applied to 'this' (issue4446070)
On Apr 29, 2011, at 1:08 PM, Basile Starynkevitch wrote: Not really. How's this then: http://stackoverflow.com/questions/2474018/when-does-invoking-a-member-function-on-a-null-instance-result-in-undefined-behav ? :-)
