selftests/ftrace: Differentiate bash and dash in dynevent_limitations.tc

[PATCH] selftests/ftrace: Differentiate bash and dash in dynevent_limitations.tc

Posted by Steven Rostedt 8 months, 1 week ago

From: Steven Rostedt <rostedt@goodmis.org>

bash and dash evaluate variables differently.
dash will evaluate '\\' every time it is read whereas bash does not.

  TEST_STRING="$TEST_STRING \\$i"
  echo $TEST_STRING

With i=123
On bash, that will print "\123"
but on dash, that will print the escape sequence of \123 as the \ will be
interpreted again in the echo.

The dynevent_limitations.tc test created a very large list of arguments to
test the maximum number of arguments to pass to the dynamic events file.
It had a loop of:

   TEST_STRING=$1
   # Acceptable
   for i in `seq 1 $MAX_ARGS`; do
     TEST_STRING="$TEST_STRING \\$i"
   done
   echo "$TEST_STRING" >> dynamic_events

This worked fine on bash, but when run on dash it failed.

This was due to dash interpreting the "\\$i" twice. Once when it was
assigned to TEST_STRING and a second time with the echo $TEST_STRING.

bash does not process the backslash more than the first time.

To solve this, assign a double backslash to a variable "bs" and then echo
it to "ts". If "ts" changes, it is dash, if not, it is bash. Then update
"bs" accordingly, and use that to assign TEST_STRING.

Now this could possibly just check if "$BASH" is defined or not, but this
is testing if the issue exists and not just which shell is being used.

Fixes: 581a7b26ab364 ("selftests/ftrace: Add dynamic events argument limitation test case")
Reported-by: Mark Brown <broonie@kernel.org>
Closes: https://lore.kernel.org/all/ccc40f2b-4b9e-4abd-8daf-d22fce2a86f0@sirena.org.uk/
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 .../test.d/dynevent/dynevent_limitations.tc   | 23 ++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/ftrace/test.d/dynevent/dynevent_limitations.tc b/tools/testing/selftests/ftrace/test.d/dynevent/dynevent_limitations.tc
index 6b94b678741a..885631c02623 100644
--- a/tools/testing/selftests/ftrace/test.d/dynevent/dynevent_limitations.tc
+++ b/tools/testing/selftests/ftrace/test.d/dynevent/dynevent_limitations.tc
@@ -7,11 +7,32 @@
 MAX_ARGS=128
 EXCEED_ARGS=$((MAX_ARGS + 1))
 
+# bash and dash evaluate variables differently.
+# dash will evaluate '\\' every time it is read whereas bash does not.
+#
+#   TEST_STRING="$TEST_STRING \\$i"
+#   echo $TEST_STRING
+#
+# With i=123
+# On bash, that will print "\123"
+# but on dash, that will print the escape sequence of \123 as the \ will
+# be interpreted again in the echo.
+#
+# Set a variable "bs" to save a double backslash, then echo that
+# to "ts" to see if $ts changed or not. If it changed, it's dash,
+# if not, it's bash, and then bs can equal a single backslash.
+bs='\\'
+ts=`echo $bs`
+if [ "$ts" = '\\' ]; then
+  # this is bash
+  bs='\'
+fi
+
 check_max_args() { # event_header
   TEST_STRING=$1
   # Acceptable
   for i in `seq 1 $MAX_ARGS`; do
-    TEST_STRING="$TEST_STRING \\$i"
+    TEST_STRING="$TEST_STRING $bs$i"
   done
   echo "$TEST_STRING" >> dynamic_events
   echo > dynamic_events
-- 
2.47.2

Re: [PATCH] selftests/ftrace: Differentiate bash and dash in dynevent_limitations.tc

Posted by Mark Brown 8 months ago

On Mon, Apr 14, 2025 at 09:09:00PM -0400, Steven Rostedt wrote:

> The dynevent_limitations.tc test created a very large list of arguments to
> test the maximum number of arguments to pass to the dynamic events file.

A bit late now since Shuah already applied this but JFTR:

Tested-by: Mark Brown <broonie@kernel.org>

Re: [PATCH] selftests/ftrace: Differentiate bash and dash in dynevent_limitations.tc

Posted by Masami Hiramatsu (Google) 8 months, 1 week ago

On Mon, 14 Apr 2025 21:09:00 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> From: Steven Rostedt <rostedt@goodmis.org>
> 
> bash and dash evaluate variables differently.
> dash will evaluate '\\' every time it is read whereas bash does not.
> 
>   TEST_STRING="$TEST_STRING \\$i"
>   echo $TEST_STRING
> 
> With i=123
> On bash, that will print "\123"
> but on dash, that will print the escape sequence of \123 as the \ will be
> interpreted again in the echo.
> 
> The dynevent_limitations.tc test created a very large list of arguments to
> test the maximum number of arguments to pass to the dynamic events file.
> It had a loop of:
> 
>    TEST_STRING=$1
>    # Acceptable
>    for i in `seq 1 $MAX_ARGS`; do
>      TEST_STRING="$TEST_STRING \\$i"
>    done
>    echo "$TEST_STRING" >> dynamic_events
> 
> This worked fine on bash, but when run on dash it failed.
> 
> This was due to dash interpreting the "\\$i" twice. Once when it was
> assigned to TEST_STRING and a second time with the echo $TEST_STRING.
> 
> bash does not process the backslash more than the first time.
> 
> To solve this, assign a double backslash to a variable "bs" and then echo
> it to "ts". If "ts" changes, it is dash, if not, it is bash. Then update
> "bs" accordingly, and use that to assign TEST_STRING.
> 
> Now this could possibly just check if "$BASH" is defined or not, but this
> is testing if the issue exists and not just which shell is being used.
> 

Thanks for fixing this issue!

Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>

Thank you!

> Fixes: 581a7b26ab364 ("selftests/ftrace: Add dynamic events argument limitation test case")
> Reported-by: Mark Brown <broonie@kernel.org>
> Closes: https://lore.kernel.org/all/ccc40f2b-4b9e-4abd-8daf-d22fce2a86f0@sirena.org.uk/
> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
> ---
>  .../test.d/dynevent/dynevent_limitations.tc   | 23 ++++++++++++++++++-
>  1 file changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/ftrace/test.d/dynevent/dynevent_limitations.tc b/tools/testing/selftests/ftrace/test.d/dynevent/dynevent_limitations.tc
> index 6b94b678741a..885631c02623 100644
> --- a/tools/testing/selftests/ftrace/test.d/dynevent/dynevent_limitations.tc
> +++ b/tools/testing/selftests/ftrace/test.d/dynevent/dynevent_limitations.tc
> @@ -7,11 +7,32 @@
>  MAX_ARGS=128
>  EXCEED_ARGS=$((MAX_ARGS + 1))
>  
> +# bash and dash evaluate variables differently.
> +# dash will evaluate '\\' every time it is read whereas bash does not.
> +#
> +#   TEST_STRING="$TEST_STRING \\$i"
> +#   echo $TEST_STRING
> +#
> +# With i=123
> +# On bash, that will print "\123"
> +# but on dash, that will print the escape sequence of \123 as the \ will
> +# be interpreted again in the echo.
> +#
> +# Set a variable "bs" to save a double backslash, then echo that
> +# to "ts" to see if $ts changed or not. If it changed, it's dash,
> +# if not, it's bash, and then bs can equal a single backslash.
> +bs='\\'
> +ts=`echo $bs`
> +if [ "$ts" = '\\' ]; then
> +  # this is bash
> +  bs='\'
> +fi
> +
>  check_max_args() { # event_header
>    TEST_STRING=$1
>    # Acceptable
>    for i in `seq 1 $MAX_ARGS`; do
> -    TEST_STRING="$TEST_STRING \\$i"
> +    TEST_STRING="$TEST_STRING $bs$i"
>    done
>    echo "$TEST_STRING" >> dynamic_events
>    echo > dynamic_events
> -- 
> 2.47.2
> 


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

Re: [PATCH] selftests/ftrace: Differentiate bash and dash in dynevent_limitations.tc

Posted by Shuah Khan 8 months, 1 week ago

On 4/15/25 16:58, Masami Hiramatsu (Google) wrote:
> On Mon, 14 Apr 2025 21:09:00 -0400
> Steven Rostedt <rostedt@goodmis.org> wrote:
> 
>> From: Steven Rostedt <rostedt@goodmis.org>
>>
>> bash and dash evaluate variables differently.
>> dash will evaluate '\\' every time it is read whereas bash does not.
>>
>>    TEST_STRING="$TEST_STRING \\$i"
>>    echo $TEST_STRING
>>
>> With i=123
>> On bash, that will print "\123"
>> but on dash, that will print the escape sequence of \123 as the \ will be
>> interpreted again in the echo.
>>
>> The dynevent_limitations.tc test created a very large list of arguments to
>> test the maximum number of arguments to pass to the dynamic events file.
>> It had a loop of:
>>
>>     TEST_STRING=$1
>>     # Acceptable
>>     for i in `seq 1 $MAX_ARGS`; do
>>       TEST_STRING="$TEST_STRING \\$i"
>>     done
>>     echo "$TEST_STRING" >> dynamic_events
>>
>> This worked fine on bash, but when run on dash it failed.
>>
>> This was due to dash interpreting the "\\$i" twice. Once when it was
>> assigned to TEST_STRING and a second time with the echo $TEST_STRING.
>>
>> bash does not process the backslash more than the first time.
>>
>> To solve this, assign a double backslash to a variable "bs" and then echo
>> it to "ts". If "ts" changes, it is dash, if not, it is bash. Then update
>> "bs" accordingly, and use that to assign TEST_STRING.
>>
>> Now this could possibly just check if "$BASH" is defined or not, but this
>> is testing if the issue exists and not just which shell is being used.
>>
> 
> Thanks for fixing this issue!
> 
> Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> 

Steve, do you want me to pick this up for rc3?

thanks,
-- Shuah

Re: [PATCH] selftests/ftrace: Differentiate bash and dash in dynevent_limitations.tc

Posted by Steven Rostedt 8 months, 1 week ago

On Tue, 15 Apr 2025 17:03:44 -0600
Shuah Khan <skhan@linuxfoundation.org> wrote:

> > Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> >   
> 
> Steve, do you want me to pick this up for rc3?

Hi Shuah,

Yes, can you please. But can you change the Closes tag to:

Closes: https://lore.kernel.org/all/350786cc-9e40-4396-ab95-4f10d69122fb@sirena.org.uk/

Because the one I had was the top of the thread which is about a
different bug. Mark mentioned this bug in the middle of the thread and
the above link is where Mark mentioned it.

-- Steve

Re: [PATCH] selftests/ftrace: Differentiate bash and dash in dynevent_limitations.tc

Posted by Shuah Khan 8 months ago

On 4/15/25 18:54, Steven Rostedt wrote:
> On Tue, 15 Apr 2025 17:03:44 -0600
> Shuah Khan <skhan@linuxfoundation.org> wrote:
> 
>>> Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
>>>    
>>
>> Steve, do you want me to pick this up for rc3?
> 
> Hi Shuah,
> 
> Yes, can you please. But can you change the Closes tag to:
> 
> Closes: https://lore.kernel.org/all/350786cc-9e40-4396-ab95-4f10d69122fb@sirena.org.uk/
> 
> Because the one I had was the top of the thread which is about a
> different bug. Mark mentioned this bug in the middle of the thread and
> the above link is where Mark mentioned it.
> 

Done. Applied to linux-kselftest fixes branch for next rc.
https://web.git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest.git/log/?h=fixes

thanks,
-- Shuah