Hi,
Ian, Wei, Anthony, can I get some feedbacks on the tools side?
Cheers,
On 05/04/2021 16:56, Julien Grall wrote:
> From: Julien Grall <jgrall@amazon.com>
>
> Hi all,
>
> By default, both Clang and GCC will happily compile C code where
> non-const char * point to literal strings. This means the following
> code will be accepted:
>
> char *str = "test";
>
> str[0] = 'a';
>
> Literal strings will reside in rodata, so they are not modifiable.
> This will result to an permission fault at runtime if the permissions
> are enforced in the page-tables (this is the case in Xen).
>
> I am not aware of code trying to modify literal strings in Xen.
> However, there is a frequent use of non-const char * to point to
> literal strings. Given the size of the codebase, there is a risk
> to involuntarily introduce code that will modify literal strings.
>
> Therefore it would be better to enforce using const when pointing
> to such strings. Both GCC and Clang provide an option to warn
> for such case (see -Wwrite-strings) and therefore could be used
> by Xen.
>
> This series doesn't yet make use of -Wwrite-strings because
> the tree is not fully converted. Instead, it contains some easy
> and likely non-controversial use const in the code.
>
> The major blockers to enable -Wwrite-strings are the following:
> - xen/common/efi: union string is used in both const and
> non-const situation. It doesn't feel right to specific one member
> const and the other non-const.
> - libxl: the major block is the flexarray framework as we would use
> it with string (now const char*). I thought it would be possible to
> make the interface const, but it looks like there are a couple of
> places where we need to modify the content (such as in
> libxl_json.c).
>
> Ideally, I would like to have -Wwrite-strings unconditionally used
> tree-wide. But, some of the area may required some heavy refactoring.
>
> One solution would be to enable it tree-wide but turned it off at a
> directroy/file level.
>
> Any opinions?
>
> Cheers,
>
> Julien Grall (14):
> xen: Constify the second parameter of rangeset_new()
> xen/sched: Constify name and opt_name in struct scheduler
> xen/x86: shadow: The return type of sh_audit_flags() should be const
> xen/char: console: Use const whenever we point to literal strings
> tools/libs: guest: Use const whenever we point to literal strings
> tools/libs: stat: Use const whenever we point to literal strings
> tools/xl: Use const whenever we point to literal strings
> tools/firmware: hvmloader: Use const in __bug() and __assert_failed()
> tools/console: Use const whenever we point to literal strings
> tools/kdd: Use const whenever we point to literal strings
> tools/misc: Use const whenever we point to literal strings
> tools/top: The string parameter in set_prompt() and set_delay() should
> be const
> tools/xenmon: xenbaked: Mark const the field text in stat_map_t
> tools/xentrace: Use const whenever we point to literal strings
>
> tools/console/client/main.c | 4 +-
> tools/console/daemon/io.c | 10 ++--
> tools/debugger/kdd/kdd.c | 10 ++--
> tools/firmware/hvmloader/util.c | 4 +-
> tools/firmware/hvmloader/util.h | 4 +-
> tools/include/xenguest.h | 10 ++--
> tools/libs/guest/xg_dom_core.c | 8 ++--
> tools/libs/guest/xg_dom_elfloader.c | 4 +-
> tools/libs/guest/xg_dom_hvmloader.c | 2 +-
> tools/libs/guest/xg_dom_x86.c | 9 ++--
> tools/libs/guest/xg_private.h | 2 +-
> tools/libs/stat/xenstat_linux.c | 4 +-
> tools/libs/stat/xenstat_qmp.c | 12 ++---
> tools/misc/xen-detect.c | 2 +-
> tools/misc/xenhypfs.c | 6 +--
> tools/xenmon/xenbaked.c | 2 +-
> tools/xentop/xentop.c | 12 ++---
> tools/xentrace/xenalyze.c | 71 +++++++++++++++--------------
> tools/xentrace/xenctx.c | 4 +-
> tools/xl/xl.h | 8 ++--
> tools/xl/xl_console.c | 2 +-
> tools/xl/xl_utils.c | 4 +-
> tools/xl/xl_utils.h | 4 +-
> xen/arch/x86/mm/shadow/multi.c | 12 ++---
> xen/common/rangeset.c | 2 +-
> xen/common/sched/private.h | 4 +-
> xen/drivers/char/console.c | 4 +-
> xen/include/xen/rangeset.h | 2 +-
> 28 files changed, 113 insertions(+), 109 deletions(-)
>
--
Julien Grall