User: Password:
|
|
Subscribe / Log in / New account

const structures

const structures

Posted Oct 6, 2011 14:07 UTC (Thu) by epa (subscriber, #39769)
In reply to: const structures by PaXTeam
Parent article: Better kernels with GCC plugins

solving the "let's make all constifiable variables const" problem is not possible with patching due to the sheer amount of patching needed.
I remember reading about this on LWN but I never quite believed it. A patch to add the 'const' keyword to some code is not hard to merge manually, even if the code has diverged a lot in the meantime. But then, the sheer size of the Linux codebase may make even a simple change unmanageable - I will defer to your experience.

I guess the answer would be to generate the patch automatically against a given source tree. Then even if only a fraction of the total patch can be applied at a time, it is possible to keep trying. Clearly if it takes a week of manual work to create the patch it's never going to be practical to get it in.


(Log in to post comments)

const structures

Posted Oct 6, 2011 19:41 UTC (Thu) by PaXTeam (subscriber, #24616) [Link]

yes, the problem with const patching is not its complexity but the amount of changes needed. i forget the exact numbers now, but a recent kernel has say 200 ops structure types (i.e., candidate for constification), and many more actual variables of said types.

the first problem you'll hit with manual patching is that it takes a lot of time to check every variable instance and their use to determine whether the given type can be made read-only or not (or you'll have to settle on individual variables only), the plugin approach is already a godsend for that reason (in the past another route was tried with coccinelle but that didn't work out too well).

the second problem is patching a given tree *and* forward porting the changes to a newer kernel, including redoing the analysis in the first step since new uses of a given type can come and go any time, meaning that the constifiable property of a type can change either way over time. the plugin approach is again the most efficient way of tracking these kind of changes.

last but not least, once you have such a plugin that can determine when to do constification, it's a very natural step to actually do it from the same plugin (it's like one extra line of code to set TREE_READONLY on the type) at which point one's enthusiasm to maintain a huge patch quickly evaporates ;).

now this was the producer side, but the consumer side is not any better unfortunately. if you read those lkml threads when constification patches were submitted, you'll realize that different developers expected them in different (and conflicting) ways (broken up by subsystem or maintainer or directory or structure type, etc), putting a huge burden on the producer side as if creating the patches wasn't consuming enough time already. then there's an issue with enforcing policy as well, seemingly noone takes constification or checkpatch.pl seriously enough as i keep seeing patches go in all the time that simply don't bother to constify structures.

all in all, while patching the source code is surely a noble goal, my and other people's lives are too short for it...

const structures

Posted Oct 7, 2011 6:26 UTC (Fri) by Lionel_Debroux (subscriber, #30014) [Link]

The constification part of the PaX patch used to represent more than 700 KB, i.e. more than one third of the raw size of the patch, which was below 2 MB. For many files touched by the patch, constification was the only type of change involved in the file.
Some constifications performed by the PaX patch were possible only due to other changes in PaX - for example, ata_port_operation, which is one of the most widely used function pointer structs in the kernel.

I myself gave up trying to push upstream the constification of snd_pcm_ops, which represented ~10% of the size of the constification part of the PaX patch, because there were so many instances to check...

Could the plugin be modified to print warnings about pointers to non-const instances being passed to functions that take pointers to const instances ? (if enabled by some argument to the plugin, because upon introduction of that feature and for quite some time afterwards, there will be thousands of occurrences...)
For instance, this could have helped against addition of mutable instances of backlight_ops being added after backlight_device_register() was modified to take a "const struct backlight_ops *ops" argument...

Finding functions that take pointers to mutable instances while they could take pointers to const instances would be harder, wouldn't it be ?

const structures

Posted Oct 7, 2011 8:22 UTC (Fri) by PaXTeam (subscriber, #24616) [Link]

> Could the plugin be modified to print warnings about pointers to non-const
> instances being passed to functions that take pointers to const instances ?

it's surely possible but i'm not sure this is what you really want as such typecasts are allowed by C and extensively used everywhere. now i assume you'd really want this detection for ops structures only in which case the problem is how the plugin would know of them (i assume you wouldn't want to use the current 'constify by default' approach). you could make use of the do_const attribute (without calling constify_type) then in the FINISH_TYPE callback you can check whether the pointed to structure is already const or not (and has the do_const attr, although on second thought, i forget now whether attrs are carried with types or have to be acted upon in the attr callback).

with all that said, i would not go the 'check the pointer' route but rather i'd constify the variable instances instead (but not function arguments) and then rely on gcc's existing warning system (if you take the address of a const object to initialize a ptr to the non-const type then you'll get a warning).


Copyright © 2017, Eklektix, Inc.
Comments and public postings are copyrighted by their creators.
Linux is a registered trademark of Linus Torvalds