On 11/16/21 1:23 PM, Jason Merrill wrote:
On 10/23/21 19:06, Martin Sebor wrote:
On 10/4/21 3:37 PM, Jason Merrill wrote:
On 10/4/21 14:42, Martin Sebor wrote:
While resolving the recent -Waddress enhancement request (PR
PR102103) I came across a 2007 problem report about GCC 4 having
stopped warning for using the address of inline functions in
equality comparisons with null. With inline functions being
commonplace in C++ this seems like an important use case for
the warning.
The change that resulted in suppressing the warning in these
cases was introduced inadvertently in a fix for PR 22252.
To restore the warning, the attached patch enhances
the decl_with_nonnull_addr_p() function to return true also for
weak symbols for which a definition has been provided.
I think you probably want to merge this function with
fold-const.c:maybe_nonzero_address, which already handles more cases.
maybe_nonzero_address() doesn't behave quite like
decl_with_nonnull_addr_p() expects and I'm reluctant to muck
around with the former too much since it's used for codegen,
while the latter just for warnings. (There is even a case
where the functions don't behave the same, and would result
in different warnings between C and C++ without some extra
help.)
So in the attached revision I just have maybe_nonzero_address()
call decl_with_nonnull_addr_p() and then refine the failing
(or uncertain) cases separately, with some overlap between
them.
Since I worked on this someone complained that some instances
of the warning newly enhanced under PR102103 aren't suppresed
in code resulting from macro expansion. Since it's trivial,
I include the fix for that report in this patch as well.
+ allocated stroage might have a null address. */
typo.
OK with that fixed.
After retesting the patch before committing I noticed it triggers
a regression in weak/weak-3.c that I missed the first time around.
Here's the test case:
extern void * ffoo1f (void);
void * foo1f (void)
{
if (ffoo1f) /* { dg-warning "-Waddress" } */
ffoo1f ();
return 0;
}
void * ffoox1f (void) { return (void *)0; }
extern void * ffoo1f (void) __attribute__((weak, alias ("ffoox1f")));
The unexpected error is:
a.c: At top level:
a.c:1:15: error: ‘ffoo1f’ declared weak after being used
1 | extern void * ffoo1f (void);
| ^~~~~~
The error is caused by the new call to maybe_nonzero_address()
made from decl_with_nonnull_addr_p(). The call registers
the symbol as used.
So unless the error is desirable for this case I think it's
best to go back to the originally proposed solution. I attach
it for reference and will plan to commit it tomorrow unless I
hear otherwise.
Martin
PS I don't know enough about the logic behind issuing this error
in other situations to tell for sure that it's wrong in this one
but I see no difference in the emitted code for a case in the same
test that declares the alias first, before taking its address and
that's accepted and this one. I also checked that both Clang and
ICC accept the code either way, so I'm inclined to think the error
would be a bug.
Restore ancient -Waddress for weak symbols [PR33925].
Resolves:
PR c/33925 - gcc -Waddress lost some useful warnings
PR c/102867 - -Waddress from macro expansion in readelf.c
gcc/c-family/ChangeLog:
PR c++/33925
PR c/102867
* c-common.c (decl_with_nonnull_addr_p): Call maybe_nonzero_address
and improve handling tof defined symbols.
gcc/c/ChangeLog:
PR c++/33925
PR c/102867
* c-typeck.c (maybe_warn_for_null_address): Suppress warnings for
code resulting from macro expansion.
gcc/cp/ChangeLog:
PR c++/33925
PR c/102867
* typeck.c (warn_for_null_address): Suppress warnings for code
resulting from macro expansion.
gcc/ChangeLog:
PR c++/33925
PR c/102867
* doc/invoke.texi (-Waddress): Update.
gcc/testsuite/ChangeLog:
PR c++/33925
PR c/102867
* g++.dg/warn/Walways-true-2.C: Adjust to avoid a valid warning.
* c-c++-common/Waddress-5.c: New test.
* c-c++-common/Waddress-6.c: New test.
* g++.dg/warn/Waddress-7.C: New test.
* g++.dg/warn/Walways-true-2.C: Adjust to avoid a valid warning.
* gcc.dg/weak/weak-3.c: Expect a warning.
diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index 436df45df68..5ab34c9eed8 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -3400,16 +3400,43 @@ c_wrap_maybe_const (tree expr, bool non_const)
/* Return whether EXPR is a declaration whose address can never be NULL.
The address of the first struct member could be NULL only if it were
- accessed through a NULL pointer, and such an access would be invalid. */
+ accessed through a NULL pointer, and such an access would be invalid.
+ The address of a weak symbol may be null unless it has a definition. */
bool
decl_with_nonnull_addr_p (const_tree expr)
{
- return (DECL_P (expr)
- && (TREE_CODE (expr) == FIELD_DECL
- || TREE_CODE (expr) == PARM_DECL
- || TREE_CODE (expr) == LABEL_DECL
- || !DECL_WEAK (expr)));
+ if (!DECL_P (expr))
+ return false;
+
+ if (TREE_CODE (expr) == FIELD_DECL
+ || TREE_CODE (expr) == PARM_DECL
+ || TREE_CODE (expr) == LABEL_DECL)
+ return true;
+
+ if (!VAR_OR_FUNCTION_DECL_P (expr))
+ return false;
+
+ if (!DECL_WEAK (expr))
+ /* Ordinary (non-weak) symbols have nonnull addresses. */
+ return true;
+
+ if (DECL_INITIAL (expr) && DECL_INITIAL (expr) != error_mark_node)
+ /* Initialized weak symbols have nonnull addresses. */
+ return true;
+
+ if (DECL_EXTERNAL (expr) || !TREE_STATIC (expr))
+ /* Uninitialized extern weak symbols and weak symbols with no
+ allocated storage might have a null address. */
+ return false;
+
+ tree attribs = DECL_ATTRIBUTES (expr);
+ if (lookup_attribute ("weakref", attribs))
+ /* Weakref symbols might have a null address unless their referent
+ is known not to. Don't bother following weakref targets here. */
+ return false;
+
+ return true;
}
/* Prepare expr to be an argument of a TRUTH_NOT_EXPR,
diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
index 782414f8c8c..50d70104766 100644
--- a/gcc/c/c-typeck.c
+++ b/gcc/c/c-typeck.c
@@ -11588,7 +11588,10 @@ build_vec_cmp (tree_code code, tree type,
static void
maybe_warn_for_null_address (location_t loc, tree op, tree_code code)
{
- if (!warn_address || warning_suppressed_p (op, OPT_Waddress))
+ /* Prevent warnings issued for macro expansion. */
+ if (!warn_address
+ || warning_suppressed_p (op, OPT_Waddress)
+ || from_macro_expansion_at (loc))
return;
if (TREE_CODE (op) == NOP_EXPR)
diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
index cb20329ceb5..58919aaf13e 100644
--- a/gcc/cp/typeck.c
+++ b/gcc/cp/typeck.c
@@ -4594,9 +4594,11 @@ build_vec_cmp (tree_code code, tree type,
static void
warn_for_null_address (location_t location, tree op, tsubst_flags_t complain)
{
+ /* Prevent warnings issued for macro expansion. */
if (!warn_address
|| (complain & tf_warning) == 0
|| c_inhibit_evaluation_warnings != 0
+ || from_macro_expansion_at (location)
|| warning_suppressed_p (op, OPT_Waddress))
return;
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 6070288856c..8a9559fa173 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -8649,6 +8649,8 @@ suppressed by casting the pointer operand to an integer type such
as @code{inptr_t} or @code{uinptr_t}.
Comparisons against string literals result in unspecified behavior
and are not portable, and suggest the intent was to call @code{strcmp}.
+The warning is suppressed if the suspicious expression is the result
+of macro expansion.
@option{-Waddress} warning is enabled by @option{-Wall}.
@item -Wno-address-of-packed-member
diff --git a/gcc/testsuite/c-c++-common/Waddress-5.c b/gcc/testsuite/c-c++-common/Waddress-5.c
new file mode 100644
index 00000000000..5d63c7dae7c
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/Waddress-5.c
@@ -0,0 +1,133 @@
+/* PR c/33925 - missing -Waddress with the address of an inline function
+ { dg-do compile }
+ { dg-options "-Wall" }
+ { dg-require-weak "" } */
+
+extern inline int eifn (void);
+extern inline int eifn_def (void) { return 0; }
+
+static inline int sifn (void);
+static inline int sifn_def (void) { return 0; }
+
+inline int ifn (void);
+inline int ifn_def (void) { return 0; }
+
+extern __attribute__ ((weak)) int ewfn (void);
+extern __attribute__ ((weak)) int ewfn_def (void) { return 0; }
+
+__attribute__ ((weak)) int wfn (void);
+__attribute__ ((weak)) int wfn_def (void) { return 0; }
+
+static __attribute__((weakref ("ewfn"))) int swrfn (void);
+
+void test_function_eqz (int *p)
+{
+ *p++ = eifn == 0; // { dg-warning "-Waddress" }
+ *p++ = eifn_def == 0; // { dg-warning "-Waddress" }
+ *p++ = sifn == 0; // { dg-warning "-Waddress" }
+ *p++ = sifn_def == 0; // { dg-warning "-Waddress" }
+ *p++ = ifn == 0; // { dg-warning "-Waddress" }
+ *p++ = ifn_def == 0; // { dg-warning "-Waddress" }
+ *p++ = ewfn == 0;
+ *p++ = ewfn_def == 0; // { dg-warning "-Waddress" }
+ *p++ = wfn == 0;
+ *p++ = wfn_def == 0; // { dg-warning "-Waddress" }
+ *p++ = swrfn == 0;
+}
+
+
+int test_function_if (int i)
+{
+ if (eifn) // { dg-warning "-Waddress" }
+ i++;
+ if (eifn_def) // { dg-warning "-Waddress" }
+ i++;
+ if (sifn) // { dg-warning "-Waddress" }
+ i++;
+ if (sifn_def) // { dg-warning "-Waddress" }
+ i++;
+ if (ifn) // { dg-warning "-Waddress" }
+ i++;
+ if (ifn_def) // { dg-warning "-Waddress" }
+ i++;
+ if (ewfn)
+ i++;
+ if (ewfn_def) // { dg-warning "-Waddress" }
+ i++;
+ if (wfn)
+ i++;
+ if(wfn_def) // { dg-warning "-Waddress" }
+ i++;
+ if (swrfn)
+ i++;
+ return i;
+}
+
+
+extern int ei;
+extern int ei_def = 1;
+
+static int si;
+static int si_def = 1;
+
+int i;
+int i_def = 1;
+
+extern __attribute__ ((weak)) int ewi; // declaration (may be null)
+extern __attribute__ ((weak)) int ewi_def = 1;
+
+__attribute__ ((weak)) int wi; // definition (cannot be bull)
+__attribute__ ((weak)) int wi_def = 1;
+
+static __attribute__((weakref ("ewi"))) int swri;
+
+void test_scalar (int *p)
+{
+ *p++ = &ei == 0; // { dg-warning "-Waddress" }
+ *p++ = &ei_def == 0; // { dg-warning "-Waddress" }
+ *p++ = &si == 0; // { dg-warning "-Waddress" }
+ *p++ = &si_def == 0; // { dg-warning "-Waddress" }
+ *p++ = &i == 0; // { dg-warning "-Waddress" }
+ *p++ = &i_def == 0; // { dg-warning "-Waddress" }
+ *p++ = &ewi == 0;
+ *p++ = &ewi_def == 0; // { dg-warning "-Waddress" }
+ *p++ = &wi == 0; // { dg-warning "-Waddress" }
+ *p++ = &wi_def == 0; // { dg-warning "-Waddress" }
+ *p++ = &swri == 0;
+}
+
+
+extern int eia[];
+extern int eia_def[] = { 1 };
+
+static int sia[1];
+static int sia_def[1] = { 1 };
+
+int ia[1];
+int ia_def[] = { 1 };
+
+extern __attribute__ ((weak)) int ewia[];
+extern __attribute__ ((weak)) int ewia_def[] = { 1 };
+
+__attribute__ ((weak)) int wia[1]; // definition (cannot be null)
+__attribute__ ((weak)) int wia_def[] = { 1 };
+
+static __attribute__((weakref ("ewia"))) int swria[1];
+
+void test_array (int *p)
+{
+ *p++ = eia == 0; // { dg-warning "-Waddress" }
+ *p++ = eia_def == 0; // { dg-warning "-Waddress" }
+ *p++ = sia == 0; // { dg-warning "-Waddress" }
+ *p++ = sia_def == 0; // { dg-warning "-Waddress" }
+ *p++ = ia == 0; // { dg-warning "-Waddress" }
+ *p++ = ia_def == 0; // { dg-warning "-Waddress" }
+ *p++ = ewia == 0;
+ *p++ = ewia_def == 0; // { dg-warning "-Waddress" }
+ *p++ = wia == 0; // { dg-warning "-Waddress" }
+ *p++ = wia_def == 0; // { dg-warning "-Waddress" }
+ *p++ = swria == 0;
+}
+
+/* { dg-prune-output "never defined" }
+ { dg-prune-output "initialized and declared 'extern'" } */
diff --git a/gcc/testsuite/c-c++-common/Waddress-6.c b/gcc/testsuite/c-c++-common/Waddress-6.c
new file mode 100644
index 00000000000..e66e6e4a0b0
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/Waddress-6.c
@@ -0,0 +1,32 @@
+/* PR c/102867 - -Waddress from macro expansion in readelf.c
+ { dg-do compile }
+ { dg-options "-Wall" } */
+
+#define F(x) ((&x) != 0)
+
+int warn_nomacro (int *p, int i)
+{
+ return &p[i] != 0; // { dg-warning "-Waddress" }
+}
+
+int nowarn_macro_expansion (int *p, int i)
+{
+ // Verify that -Waddress isn't issued for code from macro expansion.
+ return F (p[i]); // { dg-bogus "-Waddress" }
+}
+
+#define G(x, i) ((&x) + i)
+
+int warn_function_macro_expansion (int *p, int i)
+{
+ /* Verify that -Waddress is issued for code involving macro expansion
+ where the comparison takes place outside the macro. */
+ return G (*p, i) != 0; // { dg-warning "-Waddress" }
+}
+
+#define malloc __builtin_malloc
+
+int warn_object_macro_expansion (int *p, int i)
+{
+ return malloc != 0; // { dg-warning "-Waddress" }
+}
diff --git a/gcc/testsuite/g++.dg/warn/Waddress-7.C b/gcc/testsuite/g++.dg/warn/Waddress-7.C
new file mode 100644
index 00000000000..efdafa50cd1
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Waddress-7.C
@@ -0,0 +1,76 @@
+/* PR c/33925 - missing -Waddress with the address of an inline function
+ { dg-do compile }
+ { dg-options "-Wall" } */
+
+struct A
+{
+ int mf ();
+ int mf_def () { return 0; }
+
+ static int smf ();
+ static int smf_def () { return 0; }
+
+ int mi;
+ static int smi;
+ static int smi_def;
+
+ __attribute__ ((weak)) static int wsmi;
+ __attribute__ ((weak)) static int wsmi_def;
+
+ int mia[];
+ static int smia[];
+ static int smia_def[];
+
+ __attribute__ ((weak)) static int wsmia[];
+ __attribute__ ((weak)) static int wsmia_def[];
+
+ void test_waddress (bool*);
+};
+
+
+/* __attribute__ ((weak)) static */ int A::smi_def = 0;
+/* __attribute__ ((weak)) static */ int A::smia_def[] = { 0 };
+
+/* __attribute__ ((weak)) static */ int A::wsmi_def = 0;
+/* __attribute__ ((weak)) static */ int A::wsmia_def[] = { 0 };
+
+
+
+void A::test_waddress (bool *p)
+{
+ if (mf) // { dg-error "cannot convert" }
+ p++;
+ if (mf_def) // { dg-error "cannot convert" }
+ p++;
+
+ if (smf) // { dg-warning "-Waddress" }
+ p++;
+ if (smf_def) // { dg-warning "-Waddress" }
+ p++;
+
+ if (&mi) // { dg-warning "-Waddress" }
+ p++;
+ if (&smi) // { dg-warning "-Waddress" }
+ p++;
+ if (&smi_def) // { dg-warning "-Waddress" }
+ p++;
+
+ if (&wsmi)
+ p++;
+
+ if (&wsmi_def) // { dg-warning "-Waddress" }
+ p++;
+
+ if (mia) // { dg-warning "-Waddress" }
+ p++;
+ if (smia) // { dg-warning "-Waddress" }
+ p++;
+ if (smia_def) // { dg-warning "-Waddress" }
+ p++;
+
+ if (wsmia)
+ p++;
+
+ if (wsmia_def) // { dg-warning "-Waddress" }
+ p++;
+}
diff --git a/gcc/testsuite/g++.dg/warn/Walways-true-2.C b/gcc/testsuite/g++.dg/warn/Walways-true-2.C
index 29a80e5c113..e951e95b336 100644
--- a/gcc/testsuite/g++.dg/warn/Walways-true-2.C
+++ b/gcc/testsuite/g++.dg/warn/Walways-true-2.C
@@ -9,7 +9,7 @@
extern int foo (int) __attribute__ ((weak));
-int i __attribute__ ((weak));
+extern int i __attribute__ ((weak));
void
bar (int a)
diff --git a/gcc/testsuite/gcc.dg/Walways-true-2.c b/gcc/testsuite/gcc.dg/Walways-true-2.c
index 7f0bb7b10b9..ae3262b6876 100644
--- a/gcc/testsuite/gcc.dg/Walways-true-2.c
+++ b/gcc/testsuite/gcc.dg/Walways-true-2.c
@@ -9,7 +9,7 @@
extern int foo (int) __attribute__ ((weak));
-int i __attribute__ ((weak));
+extern int i __attribute__ ((weak));
void
bar (int a)
diff --git a/gcc/testsuite/gcc.dg/weak/weak-3.c b/gcc/testsuite/gcc.dg/weak/weak-3.c
index a719848c935..5fdf029cf35 100644
--- a/gcc/testsuite/gcc.dg/weak/weak-3.c
+++ b/gcc/testsuite/gcc.dg/weak/weak-3.c
@@ -55,7 +55,7 @@ void * foo1e (void)
extern void * ffoo1f (void);
void * foo1f (void)
{
- if (ffoo1f) /* { dg-warning "" } */
+ if (ffoo1f) /* { dg-warning "-Waddress" } */
ffoo1f ();
return 0;
}
@@ -68,7 +68,9 @@ void * ffoox1g (void) { return (void *)0; }
extern void * ffoo1g (void) __attribute__((weak, alias ("ffoox1g")));
void * foo1g (void)
{
- if (ffoo1g)
+ /* ffoo1g is a weak alias for a symbol defined in this file, expect
+ a -Waddress for the test (which is folded to true). */
+ if (ffoo1g) // { dg-warning "-Waddress" }
ffoo1g ();
return 0;
}
