lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Mon, 04 Jun 2018 18:18:50 -0700 (PDT) From: Palmer Dabbelt <palmer@...ive.com> To: luc.vanoostenryck@...il.com CC: yamada.masahiro@...ionext.com, Christoph Hellwig <hch@...radead.org>, linux-riscv@...ts.infradead.org, linux-kernel@...r.kernel.org, albert@...ive.com, michal.lkml@...kovi.net Subject: Re: [PATCH] riscv: pass machine size to sparse On Thu, 31 May 2018 08:35:54 PDT (-0700), luc.vanoostenryck@...il.com wrote: > On Thu, May 31, 2018 at 05:09:21AM -0700, Palmer Dabbelt wrote: >> On Mon, 28 May 2018 23:14:20 PDT (-0700), yamada.masahiro@...ionext.com wrote: >> > 2018-05-29 15:11 GMT+09:00 Christoph Hellwig <hch@...radead.org>: >> > > On Mon, May 28, 2018 at 06:35:05PM +0200, Luc Van Oostenryck wrote: >> > > > By default, sparse assumes a 64bit machine when compiled on x86-64 >> > > > and 32bit when compiled on anything else. >> > > > >> > > > This can of course create all sort of problems when this doesn't >> > > > correspond to the target's machine size, like issuing false >> > > > warnings like: 'shift too big (32) for type unsigned long' or >> > > > is 64bit while sparse was compiled on a 32bit machine, or worse, >> > > > to not emit legitimate warnings. >> > > > >> > > > Fix this by passing the appropriate -m32/-m64 flag to sparse. >> > > >> > > Can we please move this to the common Kbuild code using the >> > > CONFIG_64BIT syombol? This really should not need boiler plate in >> > > every architecture. >> > >> > >> > I agree. >> > >> > Luc did so for -mbig/little-endian: >> > https://patchwork.kernel.org/patch/10433957/ >> > >> > We should do likewise for -m32/64. >> >> Sorry for being a bit slow here, but I like the idea of making the >> 32/64-bit issue generic as it seems like it'll be necessary for >> every port. > > Sure, Christophe asked it too. > I've sent a new version doing it once and for all for all archs > but I forgot to CC you: > https://lkml.org/lkml/2018/5/30/948 > >> Looking through the patch for big/little-endian I did >> notice: >> >> * RISC-V compilers set "__riscv_xlen" to the length of an X >> (integer) register in bits. >> * RISC-V compilers define "__riscv", and it doesn't appear we inform >> sparse about that. >> >> These two might not be that interesting, but we do already have some >> cases where we're checking for __riscv_xlen in Linux. I've yet to >> successfully use sparse, but adding at least >> >> CHECKFLAGS += -D__riscv >> >> seems reasonable, > > Sure (but I don't see a dependency in the kernel (yet)). > >> and possibly also some sort of >> >> ifeq ($(CONFIG_ARCH_RV64I),y) >> CHECKFLAGS += -D__riscv_xlen=64 >> else >> CHECKFLAGS += -D__riscv_xlen=32 >> fi >> >> might be necessary. > > Yes, this one is really needed. > I'll send a patch for this one and __riscv. > >> We strive to follow the generic rules for >> ABI-related stuff like __LP64__ but I don't think there's any >> generic mapping for XLEN. Similarly there's "__riscv_flen" and >> "__riscv_float_abi_*", but those are less likely to be used by the >> kernel so they're probably not worth worrying about for now. > > Yes, I agree. > Note that sparse will define __LP64__ (and _LP64) when in -m64 mode. > >> There's also a bunch of other RISC-V macros, the only one of which >> we're currently using is "__riscv_muldiv" (and that's in a manner >> that's unlikely to trigger any sort of static analysis). Between a >> lack of Kconfig options and a glibc port we're essentially mandating >> IMA right now, so these probably don't matter. > > Yes, I just saw. I think also it's better to leave it so for now. > And if it becomes more used, then better to infer it from the compiler > than harcoding it. Makes sense. Thanks for the work!
Powered by blists - more mailing lists