procyberian/gcc
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

c, c++: attribute format on a ctor with a vbase [PR101833, PR47634]

Browse source 
Attribute format takes three arguments: archetype, string-index, and
first-to-check.  The last two specify the position in the function
parameter list.  r63030 clarified that "Since non-static C++ methods have
an implicit this argument, the arguments of such methods should be counted
from two, not one, when giving values for string-index and first-to-check."
Therefore one has to write

  struct D {
    D(const char *, ...) __attribute__((format(printf, 2, 3)));
  };

However -- and this is the problem in this PR -- ctors with virtual
bases also get two additional parameters: the in-charge parameter and
the VTT parameter (added in maybe_retrofit_in_chrg).  In fact we'll end up
with two clones of the ctor: an in-charge and a not-in-charge version (see
build_cdtor_clones).  That means that the argument position the user
specified in the attribute argument will refer to different arguments,
depending on which constructor we're currently dealing with.  This can
cause a range of problems: wrong errors, confusing warnings, or crashes.

This patch corrects that; for C we don't have to do anything, and in C++
we can use num_artificial_parms_for.  It would be wrong to rewrite the
attributes the user supplied, so I've changed POS to be passed by
reference so that we don't have to change all the call sites of
positional_argument and we still get the default_conversion adjustment.

Attribute format_arg is not affected, because it requires that the
function returns "const char *" which will never be the case for cdtors.

	PR c++/101833
	PR c++/47634

gcc/c-family/ChangeLog:

	* c-attribs.cc (positional_argument): Pass POS by reference.  Deal
	with FN being either a function declaration or function type.  Use
	maybe_adjust_arg_pos_for_attribute.
	* c-common.cc (check_function_arguments): Maybe pass FNDECL down to
	check_function_format.
	* c-common.h (maybe_adjust_arg_pos_for_attribute): Declare.
	(positional_argument): Adjust.
	* c-format.cc (get_constant): Rename to ...
	(validate_constant): ... this.  Take EXPR by reference.  Return bool
	instead of tree.
	(handle_format_arg_attribute): Don't overwrite FORMAT_NUM_EXPR by the
	return value of validate_constant.
	(decode_format_attr): Don't overwrite FORMAT_NUM_EXPR and
	FIRST_ARG_NUM_EXPR by the return value of validate_constant.
	(check_function_format): Adjust a parameter name.
	(handle_format_attribute): Maybe pass FNDECL down to decode_format_attr.

gcc/c/ChangeLog:

	* c-objc-common.cc (maybe_adjust_arg_pos_for_attribute): New.

gcc/cp/ChangeLog:

	* tree.cc (maybe_adjust_arg_pos_for_attribute): New.

gcc/ChangeLog:

	* tree-core.h (struct attribute_spec): Update comment for HANDLER.

gcc/testsuite/ChangeLog:

	* g++.dg/ext/attr-format-arg1.C: New test.
	* g++.dg/ext/attr-format1.C: New test.
	* g++.dg/ext/attr-format2.C: New test.
	* g++.dg/ext/attr-format3.C: New test.
This commit is contained in:
Marek Polacek 2022-03-31 18:31:39 -04:00
parent ea3fbfda60
commit 0c723bb4be
11 changed files with 207 additions and 55 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

33
gcc/c-family/c-attribs.cc
View file

@ -594,18 +594,23 @@ attribute_takes_identifier_p (const_tree attr_id)
}
/* Verify that argument value POS at position ARGNO to attribute NAME
applied to function TYPE refers to a function parameter at position
POS and the expected type CODE. Treat CODE == INTEGER_TYPE as
matching all C integral types except bool. If successful, return
POS after default conversions, if any. Otherwise, issue appropriate
warnings and return null. A non-zero 1-based ARGNO should be passed
in by callers only for attributes with more than one argument. */
applied to function FN (which is either a function declaration or function
type) refers to a function parameter at position POS and the expected type
CODE. Treat CODE == INTEGER_TYPE as matching all C integral types except
bool. If successful, return POS after default conversions (and possibly
adjusted by ADJUST_POS). Otherwise, issue appropriate warnings and return
null. A non-zero 1-based ARGNO should be passed in by callers only for
attributes with more than one argument.
N.B. This function modifies POS. */
tree
positional_argument (const_tree fntype, const_tree atname, tree pos,
positional_argument (const_tree fn, const_tree atname, tree &pos,
tree_code code, int argno /* = 0 */,
int flags /* = posargflags () */)
{
const_tree fndecl = TYPE_P (fn) ? NULL_TREE : fn;
const_tree fntype = TYPE_P (fn) ? fn : TREE_TYPE (fn);
if (pos && TREE_CODE (pos) != IDENTIFIER_NODE
&& TREE_CODE (pos) != FUNCTION_DECL)
pos = default_conversion (pos);
@ -682,6 +687,11 @@ positional_argument (const_tree fntype, const_tree atname, tree pos,
if (!prototype_p (fntype))
return pos;
/* ADJUST_POS is non-zero in C++ when the function type has invisible
parameters generated by the compiler, such as the in-charge or VTT
parameters. */
const int adjust_pos = maybe_adjust_arg_pos_for_attribute (fndecl);
/* Verify that the argument position does not exceed the number
of formal arguments to the function. When POSARG_ELLIPSIS
is set, ARGNO may be beyond the last argument of a vararg
@ -690,7 +700,7 @@ positional_argument (const_tree fntype, const_tree atname, tree pos,
if (!nargs
|| !tree_fits_uhwi_p (pos)
|| ((flags & POSARG_ELLIPSIS) == 0
&& !IN_RANGE (tree_to_uhwi (pos), 1, nargs)))
&& !IN_RANGE (tree_to_uhwi (pos) + adjust_pos, 1, nargs)))
{
if (argno < 1)
@ -707,8 +717,9 @@ positional_argument (const_tree fntype, const_tree atname, tree pos,
}
/* Verify that the type of the referenced formal argument matches
the expected type. */
unsigned HOST_WIDE_INT ipos = tree_to_uhwi (pos);
the expected type. Invisible parameters may have been added by
the compiler, so adjust the position accordingly. */
unsigned HOST_WIDE_INT ipos = tree_to_uhwi (pos) + adjust_pos;
/* Zero was handled above. */
gcc_assert (ipos != 0);
@ -791,7 +802,7 @@ positional_argument (const_tree fntype, const_tree atname, tree pos,
return NULL_TREE;
}
return pos;
return build_int_cst (TREE_TYPE (pos), ipos);
}
/* Return the first of DECL or TYPE attributes installed in NODE if it's

4
gcc/c-family/c-common.cc
View file

@ -6071,8 +6071,8 @@ check_function_arguments (location_t loc, const_tree fndecl, const_tree fntype,
/* Check for errors in format strings. */
if (warn_format || warn_suggest_attribute_format)
check_function_format (fntype, TYPE_ATTRIBUTES (fntype), nargs, argarray,
arglocs);
check_function_format (fndecl ? fndecl : fntype, TYPE_ATTRIBUTES (fntype), nargs,
argarray, arglocs);
if (warn_format)
check_function_sentinel (fntype, nargs, argarray);

3
gcc/c-family/c-common.h
View file

@ -1049,6 +1049,7 @@ extern tree finish_label_address_expr (tree, location_t);
extern tree lookup_label (tree);
extern tree lookup_name (tree);
extern bool lvalue_p (const_tree);
extern int maybe_adjust_arg_pos_for_attribute (const_tree);
extern bool vector_targets_convertible_p (const_tree t1, const_tree t2);
extern bool vector_types_convertible_p (const_tree t1, const_tree t2, bool emit_lax_note);
@ -1493,7 +1494,7 @@ enum posargflags {
POSARG_ELLIPSIS = 2
};
extern tree positional_argument (const_tree, const_tree, tree, tree_code,
extern tree positional_argument (const_tree, const_tree, tree &, tree_code,
int = 0, int = posargflags ());
extern enum flt_eval_method

77
gcc/c-family/c-format.cc
View file

@ -78,9 +78,9 @@ static bool check_format_string (const_tree argument,
unsigned HOST_WIDE_INT format_num,
int flags, bool *no_add_attrs,
int expected_format_type);
static tree get_constant (const_tree fntype, const_tree atname, tree expr,
int argno, unsigned HOST_WIDE_INT *value,
int flags, bool validated_p);
static bool validate_constant (const_tree fn, const_tree atname, tree &expr,
int argno, unsigned HOST_WIDE_INT *value,
int flags, bool validated_p);
static const char *convert_format_name_to_system_name (const char *attr_name);
static int first_target_format_type;
@ -172,14 +172,12 @@ handle_format_arg_attribute (tree *node, tree atname,
tree args, int flags, bool *no_add_attrs)
{
tree type = *node;
/* Note that TREE_VALUE (args) is changed in place below. */
/* Note that TREE_VALUE (args) is changed in the validate_constant call. */
tree *format_num_expr = &TREE_VALUE (args);
unsigned HOST_WIDE_INT format_num = 0;
if (tree val = get_constant (type, atname, *format_num_expr, 0, &format_num,
0, false))
*format_num_expr = val;
else
if (!validate_constant (type, atname, *format_num_expr, 0, &format_num, 0,
false))
{
*no_add_attrs = true;
return NULL_TREE;
@ -301,38 +299,39 @@ check_format_string (const_tree fntype, unsigned HOST_WIDE_INT format_num,
/* Under the control of FLAGS, verify EXPR is a valid constant that
refers to a positional argument ARGNO having a string type (char*
or, for targets like Darwin, a pointer to struct CFString) to
a function type FNTYPE declared with attribute ATNAME.
If valid, store the constant's integer value in *VALUE and return
the value.
If VALIDATED_P is true assert the validation is successful.
Returns the converted constant value on success, null otherwise. */
a function FN declared with attribute ATNAME. If valid, store the
constant's integer value in *VALUE and return true. If VALIDATED_P
is true assert the validation is successful.
static tree
get_constant (const_tree fntype, const_tree atname, tree expr, int argno,
unsigned HOST_WIDE_INT *value, int flags, bool validated_p)
N.B. This function modifies EXPR. */
static bool
validate_constant (const_tree fn, const_tree atname, tree &expr, int argno,
unsigned HOST_WIDE_INT *value, int flags, bool validated_p)
{
/* Require the referenced argument to have a string type. For targets
like Darwin, also accept pointers to struct CFString. */
if (tree val = positional_argument (fntype, atname, expr, STRING_CST,
if (tree val = positional_argument (fn, atname, expr, STRING_CST,
argno, flags))
{
*value = TREE_INT_CST_LOW (val);
return val;
return true;
}
gcc_assert (!validated_p);
return NULL_TREE;
return false;
}
/* Decode the arguments to a "format" attribute into a
function_format_info structure. It is already known that the list
is of the right length. If VALIDATED_P is true, then these
attributes have already been validated and must not be erroneous;
if false, it will give an error message. Returns true if the
attributes are successfully decoded, false otherwise. */
if false, it will give an error message. FN is either a function
declaration or function type. Returns true if the attributes are
successfully decoded, false otherwise. */
static bool
decode_format_attr (const_tree fntype, tree atname, tree args,
decode_format_attr (const_tree fn, tree atname, tree args,
function_format_info *info, bool validated_p)
{
tree format_type_id = TREE_VALUE (args);
@ -372,17 +371,13 @@ decode_format_attr (const_tree fntype, tree atname, tree args,
}
}
if (tree val = get_constant (fntype, atname, *format_num_expr,
2, &info->format_num, 0, validated_p))
*format_num_expr = val;
else
if (!validate_constant (fn, atname, *format_num_expr, 2, &info->format_num,
0, validated_p))
return false;
if (tree val = get_constant (fntype, atname, *first_arg_num_expr,
3, &info->first_arg_num,
(POSARG_ZERO | POSARG_ELLIPSIS), validated_p))
*first_arg_num_expr = val;
else
if (!validate_constant (fn, atname, *first_arg_num_expr, 3,
&info->first_arg_num,
(POSARG_ZERO | POSARG_ELLIPSIS), validated_p))
return false;
if (info->first_arg_num != 0 && info->first_arg_num <= info->format_num)
@ -1154,13 +1149,12 @@ decode_format_type (const char *s, bool *is_raw /* = NULL */)
/* Check the argument list of a call to printf, scanf, etc.
ATTRS are the attributes on the function type. There are NARGS argument
values in the array ARGARRAY.
Also, if -Wsuggest-attribute=format,
warn for calls to vprintf or vscanf in functions with no such format
attribute themselves. */
values in the array ARGARRAY. FN is either a function declaration or
function type. Also, if -Wsuggest-attribute=format, warn for calls to
vprintf or vscanf in functions with no such format attribute themselves. */
void
check_function_format (const_tree fntype, tree attrs, int nargs,
check_function_format (const_tree fn, tree attrs, int nargs,
tree *argarray, vec<location_t> *arglocs)
{
tree a;
@ -1174,7 +1168,7 @@ check_function_format (const_tree fntype, tree attrs, int nargs,
{
/* Yup; check it. */
function_format_info info;
decode_format_attr (fntype, atname, TREE_VALUE (a), &info,
decode_format_attr (fn, atname, TREE_VALUE (a), &info,
/*validated=*/true);
if (warn_format)
{
@ -5150,10 +5144,14 @@ convert_format_name_to_system_name (const char *attr_name)
/* Handle a "format" attribute; arguments as in
struct attribute_spec.handler. */
tree
handle_format_attribute (tree *node, tree atname, tree args,
handle_format_attribute (tree node[3], tree atname, tree args,
int flags, bool *no_add_attrs)
{
const_tree type = *node;
/* NODE[2] may be NULL, and it also may be a PARM_DECL for function
pointers. */
const_tree fndecl = ((node[2] && TREE_CODE (node[2]) == FUNCTION_DECL)
? node[2] : NULL_TREE);
function_format_info info;
#ifdef TARGET_FORMAT_TYPES
@ -5179,7 +5177,8 @@ handle_format_attribute (tree *node, tree atname, tree args,
if (TREE_CODE (TREE_VALUE (args)) == IDENTIFIER_NODE)
TREE_VALUE (args) = canonicalize_attr_name (TREE_VALUE (args));
if (!decode_format_attr (type, atname, args, &info, /* validated_p = */false))
if (!decode_format_attr (fndecl ? fndecl : type, atname, args, &info,
/* validated_p = */false))
{
*no_add_attrs = true;
return NULL_TREE;

9
gcc/c/c-objc-common.cc
View file

@ -394,3 +394,12 @@ c_get_alias_set (tree t)
return c_common_get_alias_set (t);
}
/* In C there are no invisible parameters like in C++ (this, in-charge, VTT,
etc.). */
int
maybe_adjust_arg_pos_for_attribute (const_tree)
{
return 0;
}

19
gcc/cp/tree.cc
View file

@ -6119,6 +6119,25 @@ maybe_warn_zero_as_null_pointer_constant (tree expr, location_t loc)
}
return false;
}
/* FNDECL is a function declaration whose type may have been altered by
adding extra parameters such as this, in-charge, or VTT. When this
takes place, the positional arguments supplied by the user (as in the
'format' attribute arguments) may refer to the wrong argument. This
function returns an integer indicating how many arguments should be
skipped. */
int
maybe_adjust_arg_pos_for_attribute (const_tree fndecl)
{
if (!fndecl)
return 0;
int n = num_artificial_parms_for (fndecl);
/* The manual states that it's the user's responsibility to account
for the implicit this parameter. */
return n > 0 ? n - 1 : 0;
}
/* Release memory we no longer need after parsing. */
void

26
gcc/testsuite/g++.dg/ext/attr-format-arg1.C Normal file
View file

@ -0,0 +1,26 @@
// PR c++/101833
// { dg-do compile }
// { dg-options "-Wall" }
struct B { };
struct V : virtual B {
const char *fmt (int, const char *) __attribute__((format_arg(3)));
};
struct D : B {
const char *fmt (int, const char *) __attribute__((format_arg(3)));
};
extern void fmt (const char *, ...) __attribute__((format(printf, 1, 2)));
void
g ()
{
V v;
fmt (v.fmt (1, "%d"), 1);
fmt (v.fmt (1, "%d"), 1lu); // { dg-warning "expects argument of type" }
D d;
fmt (d.fmt (1, "%d"), 1);
fmt (d.fmt (1, "%d"), 1lu); // { dg-warning "expects argument of type" }
}

32
gcc/testsuite/g++.dg/ext/attr-format1.C Normal file
View file

@ -0,0 +1,32 @@
// PR c++/47634
// { dg-do compile }
class Base {
public:
Base() { }
};
class VDerived : public virtual Base {
public:
VDerived(int x, const char * f, ...) __attribute__((format(printf, 3, 4)));
};
class Derived : public Base {
public:
Derived(int x, const char * f, ...) __attribute__((format(printf, 3, 4)));
};
VDerived::VDerived(int, const char *, ...)
{
}
Derived::Derived(int, const char *, ...)
{
}
int
main(int, char **)
{
throw VDerived(1, "%s %d", "foo", 1);
throw Derived(1, "%s %d", "bar", 1);
}

38
gcc/testsuite/g++.dg/ext/attr-format2.C Normal file
View file

@ -0,0 +1,38 @@
// PR c++/101833
// { dg-do compile }
// { dg-options "-Wall" }
struct B { };
struct V : virtual B {
V(int, const char *, ...) __attribute__((format(printf, 3, 4)));
};
struct D : B {
D(int, const char *, ...) __attribute__((format(printf, 3, 4)));
};
struct D2 : B {
template<typename T>
D2(T, const char *, ...) __attribute__((format(printf, 3, 4)));
};
struct V2 : virtual B {
template<typename T>
V2(T, const char *, ...) __attribute__((format(printf, 3, 4)));
};
struct X {
template<typename T>
X(T, ...) __attribute__((format(printf, 2, 3)));
};
V v(1, "%s %d", "foo", 1);
D d(1, "%s %d", "foo", 1);
D2 d2(1, "%s %d", "foo", 1);
V2 v2(1, "%s %d", "foo", 1);
// Test that it actually works.
V e1(1, "%d", 1L); // { dg-warning "expects argument of type" }
D e2(1, "%d", 1L); // { dg-warning "expects argument of type" }
X e3("%d", 1L); // { dg-warning "expects argument of type" }

15
gcc/testsuite/g++.dg/ext/attr-format3.C Normal file
View file

@ -0,0 +1,15 @@
// PR c++/101833
// { dg-do compile }
// { dg-options "-Wall" }
class Base {};
struct VDerived : virtual Base {
VDerived(int, int, const char *, ...) __attribute__((format(printf, 2, 3))); // { dg-error ".format. attribute argument 2 value .2. refers to parameter type .int." }
VDerived(int, const char *, ...) __attribute__((format(printf, 5, 6))); // { dg-warning ".format. attribute argument 2 value .5. exceeds" }
} a(1, "%s %d", "foo", 1);
struct Derived : Base {
Derived(int, int, const char *, ...) __attribute__((format(printf, 2, 3))); // { dg-error ".format. attribute argument 2 value .2. refers to parameter type .int." }
Derived(int, const char *, ...) __attribute__((format(printf, 5, 6))); // { dg-warning ".format. attribute argument 2 value .5. exceeds" }
} b(1, "%s %d", "foo", 1);

6
gcc/tree-core.h
View file

@ -2113,8 +2113,10 @@ struct attribute_spec {
bool function_type_required;
/* Specifies if attribute affects type's identity. */
bool affects_type_identity;
/* Function to handle this attribute. NODE points to the node to which
the attribute is to be applied. If a DECL, it should be modified in
/* Function to handle this attribute. NODE points to a tree[3] array,
where node[0] is the node to which the attribute is to be applied;
node[1] is the last pushed/merged declaration if one exists, and node[2]
may be the declaration for node[0]. If a DECL, it should be modified in
place; if a TYPE, a copy should be created. NAME is the canonicalized
name of the attribute i.e. without any leading or trailing underscores.
ARGS is the TREE_LIST of the arguments (which may be NULL). FLAGS gives