Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add assignment and floating-point constant attributes #171

Merged
merged 11 commits into from
Jul 30, 2024
10 changes: 10 additions & 0 deletions src/cil.ml
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,7 @@ and attrparam =
| AAddrOf of attrparam (** & a **)
| AIndex of attrparam * attrparam (** a1[a2] *)
| AQuestion of attrparam * attrparam * attrparam (** a1 ? a2 : a3 **)
| AAssign of attrparam * attrparam (** a1 = a2 *)


(** Information about a composite type (a struct or a union). Use
Expand Down Expand Up @@ -1874,6 +1875,7 @@ let additiveLevel = 60
let comparativeLevel = 70
let bitwiseLevel = 75
let questionLevel = 100
let assignLevel = 110
let getParenthLevel (e: exp) =
match e with
| Question _ -> questionLevel
Expand Down Expand Up @@ -1924,6 +1926,7 @@ let getParenthLevelAttrParam (a: attrparam) =
| AAddrOf _ -> 30
| ADot _ | AIndex _ | AStar _ -> 20
| AQuestion _ -> questionLevel
| AAssign _ -> assignLevel


let isIntegralType t =
Expand Down Expand Up @@ -4423,6 +4426,9 @@ class defaultCilPrinterClass : cilPrinter = object (self)
self#pAttrParam () a1 ++ text " ? " ++
self#pAttrParam () a2 ++ text " : " ++
self#pAttrParam () a3
| AAssign (a1, a2) ->
self#pAttrParam () a1 ++ text "=" ++
self#pAttrParam () a2


(* A general way of printing lists of attributes *)
Expand Down Expand Up @@ -5561,6 +5567,10 @@ and childrenAttrparam (vis: cilVisitor) (aa: attrparam) : attrparam =
let e3' = fAttrP e3 in
if e1' != e1 || e2' != e2 || e3' != e3
then AQuestion (e1', e2', e3') else aa
| AAssign (e1, e2) ->
let e1' = fAttrP e1 in
let e2' = fAttrP e2 in
if e1' != e1 || e2' != e2 then AAssign (e1', e2') else aa


let rec visitCilFunction (vis : cilVisitor) (f : fundec) : fundec =
Expand Down
1 change: 1 addition & 0 deletions src/cil.mli
Original file line number Diff line number Diff line change
Expand Up @@ -338,6 +338,7 @@ and attrparam =
| AAddrOf of attrparam (** & a **)
| AIndex of attrparam * attrparam (** a1[a2] *)
| AQuestion of attrparam * attrparam * attrparam (** a1 ? a2 : a3 **)
| AAssign of attrparam * attrparam (** a1 = a2 *)

(** {b Structures.} The {!compinfo} describes the definition of a
structure or union type. Each such {!compinfo} must be defined at the
Expand Down
3 changes: 3 additions & 0 deletions src/frontc/cabs2cil.ml
Original file line number Diff line number Diff line change
Expand Up @@ -2927,6 +2927,7 @@ and doAttr (a: A.attribute) : attribute list =
| _ ->
E.s (error "Invalid attribute constant: %s")
end
| A.CONSTANT (A.CONST_FLOAT str) -> ACons (str, [])
| A.CALL(A.VARIABLE n, args) -> begin
let n' = if strip then stripUnderscore n else n in
let ae' = Util.list_map ae args in
Expand All @@ -2940,6 +2941,8 @@ and doAttr (a: A.attribute) : attribute list =
ABinOp(LAnd, ae aa1, ae aa2)
| A.BINARY(A.OR, aa1, aa2) ->
ABinOp(LOr, ae aa1, ae aa2)
| A.BINARY(A.ASSIGN, aa1, aa2) ->
AAssign(ae aa1, ae aa2)
| A.BINARY(abop, aa1, aa2) ->
ABinOp (convBinOp abop, ae aa1, ae aa2)
| A.UNARY(A.PLUS, aa) -> ae aa
Expand Down
8 changes: 7 additions & 1 deletion src/frontc/cparser.mly
Original file line number Diff line number Diff line change
Expand Up @@ -1494,6 +1494,8 @@ primary_attr:
| LPAREN attr RPAREN { $2 }
| IDENT IDENT { CALL(VARIABLE (fst $1), [VARIABLE (fst $2)]) }
| CST_INT { CONSTANT(CONST_INT (fst $1)) }
| CST_FLOAT { CONSTANT(CONST_FLOAT (fst $1)) }
| CST_FLOAT CST_FLOAT { CONSTANT(CONST_FLOAT (fst $1 ^ fst $2)) } /* Clang-like hack to parse version numbers like "10.13.4" (https://github.com/goblint/cil/pull/171#issuecomment-2250670652). We lex them as "10.13" and ".4". */
| const_string_or_wstring { CONSTANT(fst $1) }
/*(* Const when it appears in
attribute lists, is translated
Expand Down Expand Up @@ -1619,8 +1621,12 @@ conditional_attr:
| logical_or_attr QUEST conditional_attr COLON conditional_attr
{ QUESTION($1, $3, $5) }

assignment_attr:
conditional_attr { $1 }
| unary_attr EQ assignment_attr { BINARY(ASSIGN, $1, $3) }

attr: conditional_attr { $1 }

attr: assignment_attr { $1 }
;

attr_list_ne:
Expand Down
3 changes: 3 additions & 0 deletions test/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,9 @@ ifdef SEPARATE
CILLY+= --nomerge
endif

# Disable GCC 14 new warnings as errors because some tests contain them
CFLAGS += -Wno-error=implicit-int -Wno-error=implicit-function-declaration -Wno-error=int-conversion -Wno-error=incompatible-pointer-types

Comment on lines +130 to +132
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens with old GCC versions? Do they already have these options?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I checked with GCC 7-13 (everything older we still look for in our RealGccConfigure) in Godbolt, that these already existed, but just weren't errors by default, so these shouldn't be a problem.
GCC 14 added a few new ones I had to exclude here (in order to avoid needing GCC-version specific configuration logic here).

# sm: this will make gcc warnings into errors; it's almost never
# what we want, but for a particular testcase (combine_copyptrs)
# I need it to show the difference between something which works
Expand Down
14 changes: 14 additions & 0 deletions test/small1/attr-assign.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// From some new MacOS headers: https://github.com/goblint/cil/issues/168

void foo(void) __attribute__((availability(macos,introduced=10.15)));

void foo(void) {
return;
}

// Version numbers may have multiple dots: https://github.com/goblint/cil/pull/171#issuecomment-2250670652
void bar(void) __attribute__((availability(macos,introduced=10.13.4)));

void bar(void) {
return;
}
2 changes: 1 addition & 1 deletion test/small2/kernel1.c
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@

// This test is nonsense: DECLARE_WAIT_QUEUE_HEAD is a macro in Linux kernel
DECLARE_WAIT_QUEUE_HEAD(log_wait);


Expand Down
3 changes: 2 additions & 1 deletion test/testcil.pl
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,7 @@ sub addToGroup {
addTest("test/attr11 _GNUCC=1");
addTest("test/attr12 _GNUCC=1");
addTest("test/attr13 _GNUCC=1");
# addTest("test/attr-assign"); # TODO: only on OSX, Linux GCC errors on introduced
addTest("testrun/packed _GNUCC=1 WARNINGS_ARE_ERRORS=1");
addTest("test/packed2 _GNUCC=1");
addTest("test/bitfield");
Expand Down Expand Up @@ -657,7 +658,7 @@ sub addToGroup {
addBadComment("scott/globalprob", "Notbug. Not a bug if fails on a non-Linux machine ;-)");
addTest("scott/bisonerror $gcc");
addTest("scott/cmpzero");
addTest("scott/kernel1 $gcc");
# addTest("scott/kernel1 $gcc");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this test no longer of interest? I would guess McPeak had a reason for adding it? In what way does it fail if it is left there?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It fails on GCC 14 with some of the new error types that aren't so easy to suppress in backwards-compatible way.

The main line of the test is

DECLARE_WAIT_QUEUE_HEAD(log_wait);

which comes from the Linux kernel, where DECLARE_WAIT_QUEUE_HEAD is supposed to be a macro (from some unknown kernel version).
This test doesn't have the macro or any includes, so this is being parsed as some very old C function declaration that declares a function without specifying its return type and also declares a new type log_wait in the middle of the function declaration.

So this test has never been about parsing actual kernel sources (which have the macro that expands to something else entirely). In it, CIL is just successfully parsing some unpreprocessed code in a completely different way from its original meaning.
CIL will still support these old C function declarations, it's just that this test isn't enabled any more.

This could be made conditional to not run on GCC 14+ (or run with different CFLAGS), but I think detecting the GCC version from gcc --version in Makefile is awfully error-prone, so it's not worth the trouble for what is a misguided test to begin with.

addTest("scott/kernel2 $gcc");
addTest("scott/xcheckers $gcc");
addTest("scott/memberofptr $gcc");
Expand Down
Loading