mainThe GNU Bourne-Again SHell - Support: sr #108720, Cluster of bugs resulting in...

 
 

sr #108720: Cluster of bugs resulting in non-execution of trap, incorrect job name.

Submitter:  Kaz Kylheku <kkylheku>
Submitted:  Thu 08 Jan 2015 10:15:23 PM UTC
   
 
Category:  None Priority:  5 - Normal
Severity:  3 - Normal Status:  Done
Privacy:  Public Assigned to:  None
Open/Closed:  Open Operating System:  GNU/Linux
* Mandatory Fields

Add a New Comment Rich Markup
   

Fri 13 Nov 2015 04:20:16 PM UTC, comment #3: 

This was fixed back in May 2015, and the fix is in bash-4.4.

Chet Ramey <chet>
Group administrator
Thu 08 Jan 2015 10:26:22 PM UTC, comment #2: 

I believe that the scenario is something like this:

  1. When the cat is interrupted, the trap executes.
  2. Because the command that is interrupted is a loop, the loop_level
     variable is nonzero, and so the wait_for function in jobs.c
     executes ADDINTERRUPT; which increments interrupt_state to nonozero.
  2. The trap logic then wants to executes the interrupt trap command
     "show_error" using parse_and_execute function in builtins/evalstring.c
     which takes that command as a plain string argument.
  3. Because the interrupt_state variable is set, parse_and_execute
     refuses to run the command. Moreover, instead of returning normally,
     it executes a throw_to_top_level.
  4. When the top level receives control, running_trap variable is still
     nonzero, and so when the next command is executed, it neglects
     to set up its name in the appropriate global variable. That
     global variable still contains the command name "cat".


I'm able to address the problem in 4 (incorrect command name) with this simple band-aid patch:

diff --git a/sig.c b/sig.c
index 3b62ea5..61dc9b0 100644
--- a/sig.c
+++ b/sig.c
@@ -59,6 +59,7 @@ extern int last_command_exit_signal;
 extern int return_catch_flag;
 extern int loop_level, continuing, breaking, funcnest;
 extern int executing_list;
+extern int running_trap;
 extern int comsub_ignore_return;
 extern int parse_and_execute_level, shell_initialized;
 #if defined (HISTORY)
@@ -443,6 +444,7 @@ throw_to_top_level ()
   run_unwind_protects ();
   loop_level = continuing = breaking = funcnest = 0;
   executing_list = comsub_ignore_return = return_catch_flag = 0;
+  running_trap = 0;

   if (interactive && print_newline)
     {


However, the real problem is that the ERR trap command is not executing because of the interrupt_state increment.

This will not happen for simple commands, which explains why when we interrupt the plain cat command (not wrapped in a loop), we see the expected error trap behavior.

That's because the wait_for function in jobs.c distinguishes loops/list execution.

Kaz Kylheku <kkylheku>
Thu 08 Jan 2015 10:19:34 PM UTC, comment #1: 

I began investigating this using "git bisect", using the original repro steps. On Bash's master branch, it looked like it was the bash-4.0-rc1 import commit which caused it, so I started bisecting through the devel branch.

Originally, on the devel branch, it looked like the bad commit is this:

commit f1c4df2484d7739a5c2ee8ae8d00614e47603b4c
Author: Chet Ramey <chet.ramey@case.edu>
Date:   Wed Dec 7 09:29:06 2011 -0500

    commit bash-20080918 snapshot


However, that was a red herring; that commit only revealed the bug with regard to the ": && cat" repro step.

I changed the repro step to "while true ; cat ; done", and then the breaking commit was found to be much older:

commit b3c50baa89649f556f4fbe4997ab0ef49d305815
Author: Chet Ramey <chet.ramey@case.edu>
Date:   Sat Dec 3 22:51:55 2011 -0500

    commit bash-20060824 snapshot


However, this is not the root cause either! This change is only related to the incorrect command name. So again, this commit only reveals the problem in that way. The real problem is elsewhere. (See my next comment.)


Kaz Kylheku <kkylheku>
Thu 08 Jan 2015 10:15:23 PM UTC, original submission:  

This was reported by Keith Thompson in the comp.unix.shell newsgroup, along with repro steps. He also reported to the Bash mailing list:

http://lists.gnu.org/archive/html/bug-bash/2015-01/msg00020.html

I have been investigating this problem, using an expect script to automate the testing,  because it involves manual steps typed into an interactive bash. My repro steps have a slight difference compared to Keith's, which is the result of some discoveries I made.

This is run in the directory where you build bash, so there is a ./bash executable.

When the behavior is incorrect, the last part of the output shows a suspended command called "cat" instead of "sleep".

There is more to the story, however.

#!/usr/bin/expect -f
spawn ./bash --norc
send "echo \$BASH_VERSION\n"
expect "$ "
send "show_error() { echo ERROR; }\n"
expect "$ "
send "trap show_error ERR\n"
expect "$ "
send "cat\n"
sleep 1
send "\003"
expect "ERROR"
expect "$ "
send "sleep 60 &\n"
expect "$ "
send "while true ; do cat ; done\n"
sleep 1
send "\003"
send "\003"
expect "$ "
send "sleep 70 &\n"
expect "$ "
send "jobs\n"
expect "$ "
send "exit\n"

Kaz Kylheku <kkylheku>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by chet (Posted a comment)
  • -email is unavailable- added by kkylheku (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

    Follows 1 latest change.

    Date Changed by Updated Field Previous Value => Replaced by
    2015-11-13 chet StatusNone Done

    Back to the top

    Powered by Savane 3.13-3230.
    Corresponding source code