| Commit message (Collapse) | Author | Age | Files | Lines |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
run_test is called by the "runtest" console command. Console commands
can take arguments, so pass along the arguments to run_test to allow
parameters to be passed to run_test.
The following command was used for automatic replacement:
git grep --name-only 'void run_test(void)' |\
xargs sed -i 's#void run_test(void)#void run_test(int argc, char **argv)##'
BRANCH=none
BUG=b:155897971
TEST=make buildall -j
TEST=Build and flash flash_write_protect test
> runtest 1
Signed-off-by: Tom Hughes <tomhughes@chromium.org>
Change-Id: Ib20b955d5ec6b98f525c94c24aadefd7a6a320a5
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/2209418
Reviewed-by: Yicheng Li <yichengli@chromium.org>
Commit-Queue: Yicheng Li <yichengli@chromium.org>
Tested-by: Yicheng Li <yichengli@chromium.org>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The semantics of %l changed during the enabling of compile-time
printf format checking. Old firmware branches will treat something
like %lx as a 64-bit value, but new code on master will enforce at
compile-time that a long (32-bits on our ECs) is passed in as the
argument. This creates a dangerous and difficult to notice situation
if the following code is cherry-picked from master into an old
firmware branch:
printf("%lx %s", myval32, mystr);
On master, this behaves correctly. On the old firmware branch, this
would swallow myval32 and mystr for %lx, and then %s would grab a
random stack pointer and print a string from it.
Deprecating %l is our mechanism for keeping such a printf from
creeping into master in the future. Obviously we can't protect against
someone that checks in code that's never tested, but anyone who tests
a printf with %l in it will notice their printf comes out with ERROR
instead of what they want.
BUG=chromium:984041
TEST=make -j buildall
BRANCH=None
Change-Id: I0267430363af7954c2ec5d2c45222759fe0ec2c1
Signed-off-by: Evan Green <evgreen@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/1834604
Reviewed-by: Jack Rosenthal <jrosenth@chromium.org>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This change removes uses of %l from the EC side of the EC codebase.
This is done because the semantics of %l within printf have changed,
and there are concerns that new calls to printf will be cherry-picked
into old firmware branches without the printf changes. So, in
preparation for disallowing %l in master, remove occurrences of %l.
This change was done by manually fixing up anything found under the EC
directory with the following regex: %[0-9*.-]*l[^l]
Remember that anything on the host machine is fine as-is, since the host
printf never changed.
BUG=chromium:984041
TEST=make -j buildall
BRANCH=None
Change-Id: I2a97433ddab5bfb8a6031ca4ff1d3905289444e2
Signed-off-by: Evan Green <evgreen@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/1834603
Reviewed-by: Jack Rosenthal <jrosenth@chromium.org>
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Add a couple basic tests for %pT values.
BUG=chromium:984041
TEST=make -j buildall
BRANCH=none
Change-Id: I8cb56017b883b6257e1432bd64eae3ae943edf8b
Signed-off-by: Evan Green <evgreen@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/1828067
Reviewed-by: Jack Rosenthal <jrosenth@chromium.org>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
When printing size_t sized integers, utilize the standard %z
modifier so that the specifier format is correct. This will enable us
to turn on compile-time printf format verification.
BUG=chromium:984041
TEST=make -j buildall
BRANCH=none
Cq-Depend:chrome-internal:1860160
Change-Id: I2c95df5c0d87677cb9fcbde33ab8846708a774a1
Signed-off-by: Evan Green <evgreen@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/1819651
Reviewed-by: Jack Rosenthal <jrosenth@chromium.org>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
In order to turn on compile-time printf format checking,
non-standard specifiers like %b (binary) must be removed. Convert
that into %pb, which takes a pointer to a structure containing the
value to print, and how many bits to print. Use the convenience
macro BINARY_VALUE() to package these values up into a struct whose
pointer is passed to printf().
Technically this is slightly more limited functionality than we used
to support given all the possible flags, field width, and precision.
However every existing instance in our codebase was using %0NNb, where
NN is some number. If more variants are needed, the parameters structure
can be expanded in the future.
BUG=chromium:984041
TEST=make -j buildall
BRANCH=None
Change-Id: I8ef995dcf97af688fbca98ab6ff59b84092b69e3
Signed-off-by: Evan Green <evgreen@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/1733100
Reviewed-by: Daisuke Nojiri <dnojiri@chromium.org>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
In order to make printf more standard, use %ph. Pass a pointer to
a struct describing the buffer, including its size. Add a convenience
macro so that conversion between the old style and new style is purely
mechanical. The old style of %h cannot be converted directly to %ph as-is
because the C standard doesn't allow flags, precision, or field width on
%p.
Ultimately the goal is to enable compile-time printf format checking.
This gets us one step closer to that.
BUG=chromium:984041
TEST=make -j buildall
BRANCH=None
Cq-Depend:chrome-internal:1559798,chrome-internal:1560598
Change-Id: I9c0ca124a048314c9b62d64bd55b36be55034e0e
Signed-off-by: Evan Green <evgreen@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/1730605
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
In order to avoid landmines later with future extensions to %p,
disallow %p by itself. The danger is that we'll have something
like: printf("%pFOO", myptr), and then later will add a %pF
extension, but miss this printf (maybe the string is split, maybe
it's just missed).
Missing a conversion during extension is worse than just seeing a
print like <ptr_val>OO, since %pF likely reaches through the
pointer and interprets its contents according to whatever F means.
Convert existing uses of %p to %pP, so they're explicitly printing
a pointer value, giving us flexibility to extend in the future.
BUG=chromium:984041
TEST=make -j buildall
BRANCH=None
Cq-Depend:chrome-internal:1560879
Change-Id: I36a4bee8d41cb9a6139171f8de0d8f2f19468132
Signed-off-by: Evan Green <evgreen@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/1730604
Reviewed-by: Jack Rosenthal <jrosenth@chromium.org>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This patch addresses a few issues with the current formatter.
The major points are as follows:
1. Cannot specify precision 0 (truncate all) for string or hexdump
2. Forced safe precision for malformed strings
3. No padding when using hexdump
4. Bad error EC_ERROR_INVAL in vsnprintf
5. Documentation errors
For (1), no piece of code explicitly sets the precision to 0 in
order to invoke the default behavior, which is currently no
precision limit.
You can check using the following grep line:
grep -rI '%[\*0-9]\{0,20\}\.0\{1,20\}[a-zA-Z]'
However, there are many cases where the precision is used to limit
the character output (as it should be).
grep -rI '%[\*0-9]\{0,20\}\.[\*0-9]\{1,20\}[a-zA-Z]'
There are many more instances that use variable precision without
checking if the precision is zero. One of which is the following:
crrev.com/4a4e2c71a0f6aaa50e0728922f84a7d54c14380a/test/host_command_fuzz.c#116
https://clusterfuzz.com/testcase-detail/5699023975088128
Our current implementation will insert ERROR and stop processing,
if a precision of zero is detected when using the hexdump flag.
This results in a badly formatted console line or runtime string,
when the intended behavior would be to simply read no bytes.
In the aforementioned fuzzer case, outputting ERROR triggers
a false positive.
Our printf should handle explicit zero precision similar to
stdlib's printf, which means truncating all the way to zero
positions, if specified.
For (2), our current implementation uses strlen to identify the
length of the input string, regardless of the set precision.
Since this is an embedded platform, we should use strnlen to
impose safe limits, when a precision is specified.
For (3), our implementation should support padding and adjusting
of all formatter types, since that is a primary feature of a
printf formatter.
The remaining commented code highlights odd behavior that should
be fixed at some point, but is not critical.
BUG=chromium:974084
TEST=Checked for any format lines that rely on a set precision of 0
grep -rI '%[\*0-9]\{0,20\}\.[\*0-9]\{1,20\}[a-zA-Z]'
TEST=make run-printf V=1
BRANCH=none
Change-Id: I897c53cce20a701fcbe8fb9572eb878817525cc3
Signed-off-by: Craig Hesling <hesling@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/1659835
Reviewed-by: Randall Spangler <rspangler@chromium.org>
|
|
These tests solidify the current behavior of the print formatter
in order to verify future changes.
Some commented out tests expose odd behavior that will be
addressed in the follow up patch.
BUG=chromium:974084
TEST=make V=1 run-printf
BRANCH=none
Change-Id: I9c45a075692992c7713a15d7f83099a2d13441e6
Signed-off-by: Craig Hesling <hesling@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/1659834
Reviewed-by: Aseda Aboagye <aaboagye@chromium.org>
|