[PATCH 0/6] modifiers inheritance by '&' and 'typeof()'

[PATCH 0/6] modifiers inheritance by '&' and 'typeof()'

[Luc Van Oostenryck]

There is several issues regarding which modifiers is or must be
preserved when using the '&' or 'typeof()' operators. There is
also no explicit tests for these. This serie aims to improve this
situation by:
  - avoid to inherit storage modifiers by '&' (patch 1)
  - simplify an existing test (patch 2 & 3)
  - create a bunch of test cases (patch 4 & 5)
  - partial fix for modifiers needing to be inherited by typeof
    (patch 6)

The last patch is marked as '[RFC]' because it needs some clarification
on the wanted semantics of some modifiers in some situations.
In particular, MOD_NODEREF & MOD_SAFE are for the moment not addressed,
like the address space.
Lastly, especially for what concerns MOD_SAFE, some others changes are
needed.

Luc Van Oostenryck (6):
  storage should not be inherited by pointers
  testsuite: simplify test function-pointer-inheritance
  use a shorter name for function-pointer-modifier-inheritance.c
  testsuite: test modifiers preserved by '&' operator
  testsuite: test modifiers preserved by 'typeof()'
  [RFC] some modifiers need to be preserved by 'typeof()'

 symbol.c                                           |   8 +-
 symbol.h                                           |   4 +-
 validation/function-pointer-inheritance.c          |   9 ++
 validation/function-pointer-modifier-inheritance.c |  18 ----
 validation/nocast.c                                |   2 +-
 validation/ptr-inherit.c                           |  80 +++++++++++++++
 validation/typeof-addresspace.c                    |  20 ++++
 validation/typeof-mods.c                           | 109 +++++++++++++++++++++
 validation/typeof-noderef.c                        |  19 ++++
 validation/typeof-safe.c                           |  23 +++++
 10 files changed, 270 insertions(+), 22 deletions(-)
 create mode 100644 validation/function-pointer-inheritance.c
 delete mode 100644 validation/function-pointer-modifier-inheritance.c
 create mode 100644 validation/ptr-inherit.c
 create mode 100644 validation/typeof-addresspace.c
 create mode 100644 validation/typeof-mods.c
 create mode 100644 validation/typeof-noderef.c
 create mode 100644 validation/typeof-safe.c

-- 
2.10.2

[PATCH 1/6] storage should not be inherited by pointers

[Luc Van Oostenryck]

Information about storage is needed for objects but once
you take the address of an object, its storage should be
irrelevant for the resulting pointer.

Trying to keep the storage into the pointer's modifiers
(while it will be available in the base type anyway) only
create corner cases later.
An example of the problem it can create is when the pointer
is dereferenced in an inlined function.

Better to simply not put have the storage informations
for the pointer, which is what this patch does.

To better illustrate the situation, suppose you have the
following variable declaration:
	static int var;

var's type should be:
	int static [toplevel] [addressable]

if you take its address the resulting pointer will be of type:
	int static [toplevel] *

while it should simply be:
	int *

Detected-by: Nicolai Stange <nicstange@gmail.com>
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 symbol.h            | 2 +-
 validation/nocast.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/symbol.h b/symbol.h
index 9b3f1604..afc4e232 100644
--- a/symbol.h
+++ b/symbol.h
@@ -247,7 +247,7 @@ struct symbol {
 #define MOD_SIZE	(MOD_CHAR | MOD_SHORT | MOD_LONG_ALL)
 #define MOD_IGNORE (MOD_TOPLEVEL | MOD_STORAGE | MOD_ADDRESSABLE |	\
 	MOD_ASSIGNED | MOD_USERTYPE | MOD_ACCESSED | MOD_EXPLICITLY_SIGNED)
-#define MOD_PTRINHERIT (MOD_VOLATILE | MOD_CONST | MOD_NODEREF | MOD_STORAGE | MOD_NORETURN | MOD_NOCAST)
+#define MOD_PTRINHERIT (MOD_VOLATILE | MOD_CONST | MOD_NODEREF | MOD_NORETURN | MOD_NOCAST)
 
 
 /* Current parsing/evaluation function */
diff --git a/validation/nocast.c b/validation/nocast.c
index c28676a3..cc0ab6b7 100644
--- a/validation/nocast.c
+++ b/validation/nocast.c
@@ -160,7 +160,7 @@ nocast.c:34:33:    got unsigned long
 nocast.c:34:33: warning: implicit cast to nocast type
 nocast.c:35:39: warning: incorrect type in initializer (different modifiers)
 nocast.c:35:39:    expected unsigned long *static [toplevel] bad_ptr_from
-nocast.c:35:39:    got unsigned long static [nocast] [toplevel] *<noident>
+nocast.c:35:39:    got unsigned long [nocast] *<noident>
 nocast.c:35:39: warning: implicit cast from nocast type
 nocast.c:50:16: warning: implicit cast from nocast type
 nocast.c:54:16: warning: implicit cast from nocast type
-- 
2.10.2

Re: [PATCH 1/6] storage should not be inherited by pointers

[Christopher Li]

On Fri, Nov 25, 2016 at 1:09 AM, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
> Information about storage is needed for objects but once
> you take the address of an object, its storage should be
> irrelevant for the resulting pointer.
>
> Trying to keep the storage into the pointer's modifiers
> (while it will be available in the base type anyway) only
> create corner cases later.

Either way it is going to be very tricky. If you make the pointer
does not inherent the object storage modifier, you need to change
all the place that assume the pointer will inherent the object storage.

Because C mostly deal with pointer, e.g. "a = b;", is actually
"*(&a) = *(&b);". Pointer is all over the place.  Right now sparse
make the pointer inherent the storage is convenient but not precise.
Changing the underlining assumption will touch a lot of code.

The extremely tricky one is the context and address space
store in "struct ctype". It is not a modifier but act like one.
Address space should belong to the storage object. But
right now address space is propagate to pointer as well.
Most of the test is done on pointer level.

> An example of the problem it can create is when the pointer
> is dereferenced in an inlined function.
>
> Better to simply not put have the storage informations
> for the pointer, which is what this patch does.

I think there will be other code changes associate with the assumption
change.

One thing to verify is if sparse issues different set of warning
on the Linux kernel check.

Chris

Re: [PATCH 1/6] storage should not be inherited by pointers

[Luc Van Oostenryck]

On Fri, Nov 25, 2016 at 10:09:25AM +0800, Christopher Li wrote:
> On Fri, Nov 25, 2016 at 1:09 AM, Luc Van Oostenryck
> <luc.vanoostenryck@gmail.com> wrote:
> > Information about storage is needed for objects but once
> > you take the address of an object, its storage should be
> > irrelevant for the resulting pointer.
> >
> > Trying to keep the storage into the pointer's modifiers
> > (while it will be available in the base type anyway) only
> > create corner cases later.
> 
> Either way it is going to be very tricky. If you make the pointer
> does not inherent the object storage modifier, you need to change
> all the place that assume the pointer will inherent the object storage.

Thing is that nowhere should sparse assume this and from what I've seen
nowhere is this assumption done.

> Because C mostly deal with pointer, e.g. "a = b;", is actually
> "*(&a) = *(&b);". Pointer is all over the place.  Right now sparse
> make the pointer inherent the storage is convenient but not precise.
> Changing the underlining assumption will touch a lot of code.
> 
> The extremely tricky one is the context and address space
> store in "struct ctype". It is not a modifier but act like one.
> Address space should belong to the storage object. But
> right now address space is propagate to pointer as well.
> Most of the test is done on pointer level.

I'm not sure we're talking about the same thing.
The patch doesn't touch to anything related to address space
which have to be inherited by the '&/adressof' operator.
The patch only concern what must be done with MOD_STATIC,
MOD_EXTERN & MOD_TOPLEVEL when we're taking the address of an object.
This is certainly one point where taking the address of an object
and then later dereferencing the pointer is *not* the same as using
directly the object, it deosn't matter anymore if the object was
static.

I didn't gave an example where the actual situation is causing a problems
but this patch is in my opinion the right solution to the problem
exposed in Nicolai's patch 20/21 (but in this case the problem only
exist because of a combinaison of a very specific case and another
deficiency) which is brievly described just here under.

> > An example of the problem it can create is when the pointer
> > is dereferenced in an inlined function.
> >
> > Better to simply not put have the storage informations
> > for the pointer, which is what this patch does.
> 
> I think there will be other code changes associate with the assumption
> change.
> 
> One thing to verify is if sparse issues different set of warning
> on the Linux kernel check.

I'll will of course carefully check this but I really doubt there will be
any problems.

Luc

Re: [PATCH 1/6] storage should not be inherited by pointers

[Christopher Li]

On Fri, Nov 25, 2016 at 11:38 AM, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
> I'm not sure we're talking about the same thing.
> The patch doesn't touch to anything related to address space
> which have to be inherited by the '&/adressof' operator.
> The patch only concern what must be done with MOD_STATIC,
> MOD_EXTERN & MOD_TOPLEVEL when we're taking the address of an object.
> This is certainly one point where taking the address of an object
> and then later dereferencing the pointer is *not* the same as using
> directly the object, it deosn't matter anymore if the object was
> static.

Sorry I jump the conclusion. If it is only concern with MOD_STATIC,
 MOD_EXTERN & MOD_TOPLEVEL, that is not a bug issue at all.
I thought you are going to extend that logic to other modifier bits as well.

Chris

Re: [PATCH 1/6] storage should not be inherited by pointers

[Luc Van Oostenryck]

On Mon, Nov 28, 2016 at 09:04:56AM +0800, Christopher Li wrote:
> Sorry I jump the conclusion. If it is only concern with MOD_STATIC,
>  MOD_EXTERN & MOD_TOPLEVEL, that is not a bug issue at all.
> I thought you are going to extend that logic to other modifier bits as well.

Well ... yes and no :)
Like it can be seen in the other patches, I mostly just
wrote some tests that, I think, clearly expose soem problems.
Those problems can be considered as bugs or not, some clearly are,
I think, some much less so but trigger questions.

I think we should clearly define what is the desired behavior of
modifiers like nocast, noderef, bitwise, ... when
- taking the address of an object with such modifiers
- using typeof() on such object or pointers
- assignment with them (but this seems much better done/defined)

The case with 'safe' is similar but its working is a bit different.

And indeed, like I think you fear, any changes regarding the
address_space will create problems in existing code and
it's why I think it's better for now to first look at the situation
with the modifiers.

You certainly can see this seriemore  as the start of a reflexion
than a try to solve anything.

Luc

[PATCH 2/6] testsuite: simplify test function-pointer-inheritance

[Luc Van Oostenryck]

The function used had so much args that it was hard
to see what's the difference between them.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 validation/function-pointer-modifier-inheritance.c | 13 ++-----------
 1 file changed, 2 insertions(+), 11 deletions(-)

diff --git a/validation/function-pointer-modifier-inheritance.c b/validation/function-pointer-modifier-inheritance.c
index 3428715a..13df6ff7 100644
--- a/validation/function-pointer-modifier-inheritance.c
+++ b/validation/function-pointer-modifier-inheritance.c
@@ -1,15 +1,6 @@
-struct sk_buff;
-struct sock;
+extern int foo(int f(int, void *));
 
-extern int skb_append_datato_frags(struct sock *sk, struct sk_buff *skb,
-                    int getfrag(void *from, char *to, int offset,
-                    int len,int odd, struct sk_buff *skb),
-                    void *from, int length);

[PATCH 3/6] use a shorter name for function-pointer-modifier-inheritance.c

[Luc Van Oostenryck]

The test file name is a bit longish which is annoying in test reports.
Rename it to simply 'function-pointer-inheritance.c'

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 ...on-pointer-modifier-inheritance.c => function-pointer-inheritance.c} | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
 rename validation/{function-pointer-modifier-inheritance.c => function-pointer-inheritance.c} (63%)

diff --git a/validation/function-pointer-modifier-inheritance.c b/validation/function-pointer-inheritance.c
similarity index 63%
rename from validation/function-pointer-modifier-inheritance.c
rename to validation/function-pointer-inheritance.c
index 13df6ff7..0b24e458 100644
--- a/validation/function-pointer-modifier-inheritance.c
+++ b/validation/function-pointer-inheritance.c
@@ -5,5 +5,5 @@ int foo(int (*f)(int, void *))
     return 0;
 }
 /*
- * check-name: Function pointer modifier inheritance
+ * check-name: Function pointer inheritance
  */
-- 
2.10.2

[PATCH 4/6] testsuite: test modifiers preserved by '&' operator

[Luc Van Oostenryck]

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 validation/ptr-inherit.c | 80 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 80 insertions(+)
 create mode 100644 validation/ptr-inherit.c

diff --git a/validation/ptr-inherit.c b/validation/ptr-inherit.c
new file mode 100644
index 00000000..58524a71
--- /dev/null
+++ b/validation/ptr-inherit.c
@@ -0,0 +1,80 @@
+#define	__user		__attribute__((address_space(1)))
+#define	__noderef	__attribute__((noderef))
+#define	__bitwise	__attribute__((bitwise))
+#define	__nocast	__attribute__((nocast))
+#define	__safe		__attribute__((safe))
+
+
+/* Should be inherited? */
+static void test_const(void)
+{
+	const int o;
+	int *p = &o;			/* check-should-fail */
+}
+
+static void test_volatile(void)
+{
+	volatile int o;
+	int *p = &o;			/* check-should-fail */
+}
+
+static void test_noderef(void)
+{
+	int __noderef o;
+	int *p = &o;			/* check-should-fail */
+}
+
+static void test_bitwise(void)
+{
+	int __bitwise o;
+	int *p = &o;			/* check-should-fail */
+}
+
+static void test_user(void)
+{
+	int __user o;
+	int *p = &o;			/* check-should-fail */
+}
+
+static void test_nocast(void)
+{
+	int __nocast o;
+	int __nocast *p = &o;		/* check-should-pass */
+}
+
+/* Should be ignored? */
+static void test_static(void)
+{
+	/* storage is not inherited */
+	static int o;
+	int *p = &o;			/* check-should-pass */
+}
+
+static void test_tls(void)
+{
+	/* storage is not inherited */
+	static __thread int o;
+	int *p = &o;			/* check-should-pass */
+}
+
+/*
+ * check-name: ptr-inherit.c
+ *
+ * check-error-start
+ptr-inherit.c:12:19: warning: incorrect type in initializer (different modifiers)
+ptr-inherit.c:12:19:    expected int *p
+ptr-inherit.c:12:19:    got int const *<noident>
+ptr-inherit.c:18:19: warning: incorrect type in initializer (different modifiers)
+ptr-inherit.c:18:19:    expected int *p
+ptr-inherit.c:18:19:    got int volatile *<noident>
+ptr-inherit.c:24:19: warning: incorrect type in initializer (different modifiers)
+ptr-inherit.c:24:19:    expected int *p
+ptr-inherit.c:24:19:    got int [noderef] *<noident>
+ptr-inherit.c:30:19: warning: incorrect type in initializer (different base types)
+ptr-inherit.c:30:19:    expected int *p
+ptr-inherit.c:30:19:    got restricted int *<noident>
+ptr-inherit.c:36:19: warning: incorrect type in initializer (different address spaces)
+ptr-inherit.c:36:19:    expected int *p
+ptr-inherit.c:36:19:    got int <asn:1>*<noident>
+ * check-error-end
+ */
-- 
2.10.2

[PATCH 5/6] testsuite: test modifiers preserved by 'typeof()'

[Luc Van Oostenryck]

Note: address space, 'safe' & 'noderef' have their own test file
because they have others issues or need to be handled differently.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 validation/typeof-addresspace.c |  20 ++++++++
 validation/typeof-mods.c        | 109 ++++++++++++++++++++++++++++++++++++++++
 validation/typeof-noderef.c     |  19 +++++++
 validation/typeof-safe.c        |  23 +++++++++
 4 files changed, 171 insertions(+)
 create mode 100644 validation/typeof-addresspace.c
 create mode 100644 validation/typeof-mods.c
 create mode 100644 validation/typeof-noderef.c
 create mode 100644 validation/typeof-safe.c

diff --git a/validation/typeof-addresspace.c b/validation/typeof-addresspace.c
new file mode 100644
index 00000000..a94f77a3
--- /dev/null
+++ b/validation/typeof-addresspace.c
@@ -0,0 +1,20 @@
+#define	__as		__attribute__((address_space(1)))
+
+static void test_as(void)
+{
+	int __as obj, *ptr;
+	typeof(obj) var = obj;
+	typeof(ptr) ptr2 = ptr;
+	typeof(*ptr) var2 = obj;
+	typeof(*ptr) *ptr3 = ptr;	/* check-should-pass */
+	typeof(obj) *ptr4 = ptr;	/* check-should-pass */
+	obj = obj;
+	ptr = ptr;
+	ptr = &obj;
+	obj = *ptr;
+}
+
+/*
+ * check-name: typeof-addresspace.c
+ * check-known-to-fail
+ */
diff --git a/validation/typeof-mods.c b/validation/typeof-mods.c
new file mode 100644
index 00000000..8c98ab8c
--- /dev/null
+++ b/validation/typeof-mods.c
@@ -0,0 +1,109 @@
+#define	__noderef	__attribute__((noderef))
+#define	__bitwise	__attribute__((bitwise))
+#define	__nocast	__attribute__((nocast))
+#define	__safe		__attribute__((safe))
+
+static void test_spec(void)
+{
+	unsigned int obj, *ptr;
+	typeof(obj) var = obj;
+	typeof(ptr) ptr2 = ptr;
+	typeof(*ptr) var2 = obj;
+	typeof(*ptr) *ptr3 = ptr;
+	typeof(obj) *ptr4 = ptr;
+	obj = obj;
+	ptr = ptr;
+	ptr = &obj;
+	obj = *ptr;
+}
+
+static void test_const(void)
+{
+	const int obj, *ptr;
+	typeof(obj) var = obj;
+	typeof(ptr) ptr2 = ptr;
+	typeof(*ptr) var2 = obj;
+	typeof(*ptr) *ptr3 = ptr;
+	typeof(obj) *ptr4 = ptr;
+	ptr = ptr;
+	ptr = &obj;
+}
+
+static void test_volatile(void)
+{
+	volatile int obj, *ptr;
+	typeof(obj) var = obj;
+	typeof(ptr) ptr2 = ptr;
+	typeof(*ptr) var2 = obj;
+	typeof(*ptr) *ptr3 = ptr;
+	typeof(obj) *ptr4 = ptr;
+	obj = obj;
+	ptr = ptr;
+	ptr = &obj;
+	obj = *ptr;
+}
+
+static void test_bitwise(void)
+{
+	typedef int __bitwise type_t;
+	type_t obj, *ptr;
+	typeof(obj) var = obj;
+	typeof(ptr) ptr2 = ptr;
+	typeof(*ptr) var2 = obj;
+	typeof(*ptr) *ptr3 = ptr;
+	typeof(obj) *ptr4 = ptr;
+	obj = obj;
+	ptr = ptr;
+	ptr = &obj;
+	obj = *ptr;
+}
+
+static void test_static(void)
+{
+	static int obj, *ptr;
+	typeof(obj) var = obj;
+	typeof(ptr) ptr2 = ptr;
+	typeof(*ptr) var2 = obj;
+	typeof(*ptr) *ptr3 = ptr;
+	typeof(obj) *ptr4 = ptr;
+	obj = obj;
+	ptr = ptr;
+	ptr = &obj;
+	obj = *ptr;
+}
+
+static void test_tls(void)
+{
+	__thread int obj, *ptr;
+	typeof(obj) var = obj;
+	typeof(ptr) ptr2 = ptr;
+	typeof(*ptr) var2 = obj;
+	typeof(*ptr) *ptr3 = ptr;
+	typeof(obj) *ptr4 = ptr;
+	obj = obj;
+	ptr = ptr;
+	ptr = &obj;
+	obj = *ptr;
+}
+
+static void test_nocast(void)
+{
+	int __nocast obj, *ptr;
+	typeof(obj) var = obj;
+	typeof(ptr) ptr2 = ptr;
+	typeof(*ptr) var2 = obj;
+	typeof(*ptr) *ptr3 = ptr;
+	typeof(obj) *ptr4 = ptr;
+	obj = obj;
+	ptr = ptr;
+	ptr = &obj;
+	obj = *ptr;
+}
+
+/*
+ * check-name: typeof-mods
+ * check-known-to-fail
+ *
+ * check-error-start
+ * check-error-end
+ */
diff --git a/validation/typeof-noderef.c b/validation/typeof-noderef.c
new file mode 100644
index 00000000..e95a53ad
--- /dev/null
+++ b/validation/typeof-noderef.c
@@ -0,0 +1,19 @@
+#define	__noderef	__attribute__((noderef))
+
+static void test_noderef(void)
+{
+	int __noderef obj, *ptr;
+	typeof(ptr) ptr2 = ptr;
+	typeof(*ptr) *ptr3 = ptr;
+	typeof(obj) *ptr4 = ptr;
+	ptr = ptr;
+	ptr = &obj;
+}
+
+/*
+ * check-name: typeof-noderef
+ * check-known-to-fail
+ *
+ * check-error-start
+ * check-error-end
+ */
diff --git a/validation/typeof-safe.c b/validation/typeof-safe.c
new file mode 100644
index 00000000..614863fb
--- /dev/null
+++ b/validation/typeof-safe.c
@@ -0,0 +1,23 @@
+#define	__safe		__attribute__((safe))
+
+static void test_safe(void)
+{
+	int __safe obj, *ptr;
+	typeof(obj) var = obj;
+	typeof(ptr) ptr2 = ptr;
+	typeof(*ptr) var2 = obj;
+	typeof(*ptr) *ptr3 = ptr;
+	typeof(obj) *ptr4 = ptr;
+	obj = obj;
+	ptr = ptr;
+	ptr = &obj;
+	obj = *ptr;
+}
+
+/*
+ * check-name: typeof-safe
+ * check-known-to-fail
+ *
+ * check-error-start
+ * check-error-end
+ */
-- 
2.10.2

[PATCH 6/6] [RFC] some modifiers need to be preserved by 'typeof()'

[Luc Van Oostenryck]

Currently when we use typeof() all the modifiers from the source
type are ignored and thus not present in the resulting type.

This give all sort of problems. One simple example is:
	const int obj;
	typeof(obj) *ptr;
	*ptr = 0;

We can expect that 'ptr' will have the type 'const int *' and thus
that sparse will warn on the assignment in the last line but it's
not case because 'ptr' has in fact the type 'int *'.

The patch fix this by preserving some of the modifiers when using
typeof().

WARNING: it may be that the old behaviour was so more or less on
  purpose as it provide a way to remove some modifiers by casting
  while still being able to use generic macros. For exmaple, it
  was possible to write the following macro:
	#define noconst(x) (typeof(x))
  and use 'noconst()' as a cast to remove 'const' from types.
  Of course, the problem with this macros is that it remove
  *all* modifiers, not only 'const'.
  With the patch, it won't be possible anymore to do this sort of
  things, which maybe is fine for 'const' but it's why MOD_NOREFREF
  is still dropped as it create problems in the Linew kernel, for example
  in the macro to convert back a percpu variable into a plain one.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 symbol.c                 | 8 ++++++--
 symbol.h                 | 2 ++
 validation/typeof-mods.c | 1 -
 3 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/symbol.c b/symbol.c
index 92a7a625..a65ad17b 100644
--- a/symbol.c
+++ b/symbol.c
@@ -466,12 +466,16 @@ struct symbol *examine_symbol_type(struct symbol * sym)
 	case SYM_TYPEOF: {
 		struct symbol *base = evaluate_expression(sym->initializer);
 		if (base) {
+			unsigned long mod = 0;
+
 			if (is_bitfield_type(base))
 				warning(base->pos, "typeof applied to bitfield type");
-			if (base->type == SYM_NODE)
+			if (base->type == SYM_NODE) {
+				mod |= base->ctype.modifiers & MOD_TYPEOF;
 				base = base->ctype.base_type;
+			}
 			sym->type = SYM_NODE;
-			sym->ctype.modifiers = 0;
+			sym->ctype.modifiers = mod;
 			sym->ctype.base_type = base;
 			return examine_node_type(sym);
 		}
diff --git a/symbol.h b/symbol.h
index afc4e232..51be81fb 100644
--- a/symbol.h
+++ b/symbol.h
@@ -248,6 +248,8 @@ struct symbol {
 #define MOD_IGNORE (MOD_TOPLEVEL | MOD_STORAGE | MOD_ADDRESSABLE |	\
 	MOD_ASSIGNED | MOD_USERTYPE | MOD_ACCESSED | MOD_EXPLICITLY_SIGNED)
 #define MOD_PTRINHERIT (MOD_VOLATILE | MOD_CONST | MOD_NODEREF | MOD_NORETURN | MOD_NOCAST)
+/* modifiers preserved by typeof() operator */
+#define MOD_TYPEOF	(MOD_VOLATILE | MOD_CONST | MOD_NOCAST | MOD_SPECIFIER)
 
 
 /* Current parsing/evaluation function */
diff --git a/validation/typeof-mods.c b/validation/typeof-mods.c
index 8c98ab8c..9822e96f 100644
--- a/validation/typeof-mods.c
+++ b/validation/typeof-mods.c
@@ -102,7 +102,6 @@ static void test_nocast(void)
 
 /*
  * check-name: typeof-mods
- * check-known-to-fail
  *
  * check-error-start
  * check-error-end
-- 
2.10.2