linux-kernel - Re: [PATCH v3] perf probe: Support signedness casting

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Thu, 11 Aug 2016 04:37:45 +0900
From:	Masami Hiramatsu <mhiramat@...nel.org>
To:	Arnaldo Carvalho de Melo <acme@...nel.org>
Cc:	Naohiro Aota <naohiro.aota@...t.com>,
	Peter Zijlstra <peterz@...radead.org>,
	Ingo Molnar <mingo@...hat.com>,
	Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
	Wang Nan <wangnan0@...wei.com>,
	Hemant Kumar <hemant@...ux.vnet.ibm.com>,
	linux-kernel@...r.kernel.org, Steven Rostedt <rostedt@...dmis.org>
Subject: Re: [PATCH v3] perf probe: Support signedness casting

On Wed, 10 Aug 2016 10:04:40 -0300
Arnaldo Carvalho de Melo <acme@...nel.org> wrote:

> Em Wed, Aug 10, 2016 at 07:38:28AM +0900, Masami Hiramatsu escreveu:
> > On Tue, 9 Aug 2016 11:05:28 -0300 Arnaldo Carvalho de Melo <acme@...nel.org> wrote:
> > > Em Tue, Aug 09, 2016 at 11:40:08AM +0900, Naohiro Aota escreveu:
> > > > This patch add signedness casting support. By specifying "s" or "u" as a
> > > > type, perf-probe will investigate variable size as usual and use
> > > > the specified signedness.
> 
> > > Humm, I tried with :u and got hexadecimal numbers, as before :-\ Can't
> > > we do decimal numbers when :u is used? Just like with :s. We could then
> > > use nothing and get the current behaviour or use :x for hexadecimal
> > > numbers.
> > 
> > Hmm, would you mean we'll change the format in ftrace or perf?
> > u8/16/32/64 is already used for showing hexadecimal numbers, and
> > how it is shown, is decided by lib/traceevent. I think we can add
> > specifying format string as a option in addition to the type cast for
> > ftrace. But for perf, I'm not sure how it is decided to show the data.
> > Does it follow the ftrace's printf format?
> 
> Humm, what I asked was: why using :s makes it appears as signed decimal
> while :u makes it appear as unsigned _hexa_decimal?

IIRC, I decided that. Since I would like to use kprobes mainly for debug,
show vars in hexadecimal by default(like debug dump). But for the signed
value the decimal should be used (%x implies unsigned).
Actually, without any type casting, kprobe argument is printed in hexadecimal
by default (because it is just a dump of register/memory). So, signed value
is special.

> Someone reading the announce for this patch is lead to think that 's'
> and 'u' are just for signedness. So having another letter ('x') to
> specify how to format it seems like a valid expectation.

I see, I think when we start supporting debuginfo by perf, I should have
come up with that. At this point, I'm just considering backward compatibility
and extensibility, since if 'u' means decimal and 'x' means hexadecimal,
'u8','u16','u32','u64' should also be changed to decimal. And also, we might
consider the case if someone asks to show vars in octal (like file permission)
or in fixed-width zero-filled hexadecimal (like address). It seems not so much
variety, so it may be possible to introduce 'oXX' for octal and 'p' for address
etc. (but is that type casting...?)

> If ftrace would use it? I don't know, I think it should.
> 
> And perf currently uses libtraceevent to do most of the field pretty
> printing (symbol resolving is done using perf's symbol.c and friends,
> for instance).

Hm, OK, it seems using ftrace format string, so I think I just need to
change it.

> 
> - Arnaldo 
>  
> > > Anyway, applied, when using :s this is a nice improvement, thanks!

-- 
Masami Hiramatsu <mhiramat@...nel.org>