Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(3365)

Issue 311350043: Add test linux.sigaction0, testing sigaction syscall without signals. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
7 years, 4 months ago by Edmund.Grimley.Evans
Modified:
7 years, 4 months ago
Reviewers:
zhaoqin, bruening
CC:
dynamorio-devs_googlegroups.com
Visibility:
Public.

Description

Commit log for first patchset: --------------- Add test linux.sigaction0, testing sigaction syscall without signals. The "sigaction" system call is fairly complex, even in the absence of signals, and should be emulated accurately by DynamoRIO. The test is disabled as it does not yet pass. ---------------

Patch Set 1 #

Total comments: 5

Patch Set 2 : print #

Total comments: 36

Patch Set 3 : Renaming #

Patch Set 4 : Rename files #

Total comments: 4

Patch Set 5 : Committed #

Unified diffs Side-by-side diffs Delta from patch set Stats (+382 lines, --1 lines) Patch
M suite/tests/CMakeLists.txt View 1 2 3 4 1 chunk +3 lines, -0 lines 0 comments Download
A suite/tests/linux/sigaction_nosignals.c View 1 2 3 4 1 chunk +380 lines, -0 lines 0 comments Download
A + suite/tests/linux/sigaction_nosignals.expect View 1 2 3 0 chunks +-1 lines, --1 lines 0 comments Download

Messages

Total messages: 19
Edmund.Grimley.Evans
7 years, 4 months ago (2016-11-30 10:24:29 UTC) #1
bruening
Why "linux.sigaction0" instead of just "linux.sigaction"?
7 years, 4 months ago (2016-12-02 15:53:35 UTC) #2
zhaoqin
need use print instead of printf. https://codereview.appspot.com/311350043/diff/1/suite/tests/linux/sigaction0.c File suite/tests/linux/sigaction0.c (right): https://codereview.appspot.com/311350043/diff/1/suite/tests/linux/sigaction0.c#newcode360 suite/tests/linux/sigaction0.c:360: printf("all done\n"); we ...
7 years, 4 months ago (2016-12-02 16:28:46 UTC) #3
Edmund.Grimley.Evans
https://codereview.appspot.com/311350043/diff/1/suite/tests/linux/sigaction0.c File suite/tests/linux/sigaction0.c (right): https://codereview.appspot.com/311350043/diff/1/suite/tests/linux/sigaction0.c#newcode360 suite/tests/linux/sigaction0.c:360: printf("all done\n"); On 2016/12/02 16:28:46, zhaoqin wrote: > we ...
7 years, 4 months ago (2016-12-02 16:56:39 UTC) #4
bruening
https://codereview.appspot.com/311350043/diff/1/suite/tests/linux/sigaction0.c File suite/tests/linux/sigaction0.c (right): https://codereview.appspot.com/311350043/diff/1/suite/tests/linux/sigaction0.c#newcode360 suite/tests/linux/sigaction0.c:360: printf("all done\n"); On 2016/12/02 16:56:39, Edmund.Grimley.Evans wrote: > On ...
7 years, 4 months ago (2016-12-02 17:03:35 UTC) #5
Edmund.Grimley.Evans
https://codereview.appspot.com/311350043/diff/1/suite/tests/linux/sigaction0.c File suite/tests/linux/sigaction0.c (right): https://codereview.appspot.com/311350043/diff/1/suite/tests/linux/sigaction0.c#newcode360 suite/tests/linux/sigaction0.c:360: printf("all done\n"); > Using printf is a source of ...
7 years, 4 months ago (2016-12-02 17:16:54 UTC) #6
Edmund.Grimley.Evans
Commit log for latest patchset: --------------- Add test linux.sigaction0, testing sigaction syscall without signals. The ...
7 years, 4 months ago (2016-12-02 17:27:20 UTC) #7
bruening
https://codereview.appspot.com/311350043/diff/1/suite/tests/linux/sigaction0.c File suite/tests/linux/sigaction0.c (right): https://codereview.appspot.com/311350043/diff/1/suite/tests/linux/sigaction0.c#newcode360 suite/tests/linux/sigaction0.c:360: printf("all done\n"); On 2016/12/02 17:16:54, Edmund.Grimley.Evans wrote: > > ...
7 years, 4 months ago (2016-12-02 22:51:55 UTC) #8
zhaoqin
https://codereview.appspot.com/311350043/diff/20001/suite/tests/linux/sigaction0.c File suite/tests/linux/sigaction0.c (right): https://codereview.appspot.com/311350043/diff/20001/suite/tests/linux/sigaction0.c#newcode49 suite/tests/linux/sigaction0.c:49: #define SIGSETSIZE 8 /* size of sigset mask */ ...
7 years, 4 months ago (2016-12-05 16:50:39 UTC) #9
Edmund.Grimley.Evans
https://codereview.appspot.com/311350043/diff/20001/suite/tests/linux/sigaction0.c File suite/tests/linux/sigaction0.c (right): https://codereview.appspot.com/311350043/diff/20001/suite/tests/linux/sigaction0.c#newcode49 suite/tests/linux/sigaction0.c:49: #define SIGSETSIZE 8 /* size of sigset mask */ ...
7 years, 4 months ago (2016-12-05 17:38:29 UTC) #10
bruening
https://codereview.appspot.com/311350043/diff/20001/suite/tests/linux/sigaction0.c File suite/tests/linux/sigaction0.c (right): https://codereview.appspot.com/311350043/diff/20001/suite/tests/linux/sigaction0.c#newcode33 suite/tests/linux/sigaction0.c:33: /* Test sigaction without signals. */ Is that what ...
7 years, 4 months ago (2016-12-05 17:41:39 UTC) #11
Edmund.Grimley.Evans
https://codereview.appspot.com/311350043/diff/20001/suite/tests/linux/sigaction0.c File suite/tests/linux/sigaction0.c (right): https://codereview.appspot.com/311350043/diff/20001/suite/tests/linux/sigaction0.c#newcode33 suite/tests/linux/sigaction0.c:33: /* Test sigaction without signals. */ On 2016/12/05 17:41:39, ...
7 years, 4 months ago (2016-12-05 19:42:50 UTC) #12
bruening
https://codereview.appspot.com/311350043/diff/20001/suite/tests/linux/sigaction0.c File suite/tests/linux/sigaction0.c (right): https://codereview.appspot.com/311350043/diff/20001/suite/tests/linux/sigaction0.c#newcode49 suite/tests/linux/sigaction0.c:49: #define SIGSETSIZE 8 /* size of sigset mask */ ...
7 years, 4 months ago (2016-12-05 20:26:36 UTC) #13
zhaoqin
https://codereview.appspot.com/311350043/diff/20001/suite/tests/linux/sigaction0.c File suite/tests/linux/sigaction0.c (right): https://codereview.appspot.com/311350043/diff/20001/suite/tests/linux/sigaction0.c#newcode49 suite/tests/linux/sigaction0.c:49: #define SIGSETSIZE 8 /* size of sigset mask */ ...
7 years, 4 months ago (2016-12-05 20:50:21 UTC) #14
Edmund.Grimley.Evans
https://codereview.appspot.com/311350043/diff/20001/suite/tests/linux/sigaction0.c File suite/tests/linux/sigaction0.c (right): https://codereview.appspot.com/311350043/diff/20001/suite/tests/linux/sigaction0.c#newcode33 suite/tests/linux/sigaction0.c:33: /* Test sigaction without signals. */ On 2016/12/05 17:41:39, ...
7 years, 4 months ago (2016-12-06 12:41:09 UTC) #15
Edmund.Grimley.Evans
Commit log for latest patchset: --------------- Add test linux.sigaction0, testing sigaction syscall without signals. The ...
7 years, 4 months ago (2016-12-06 12:44:01 UTC) #16
Edmund.Grimley.Evans
Commit log for latest patchset: --------------- Add test linux.sigaction_nosignals, testing syscall without signals. The "sigaction" ...
7 years, 4 months ago (2016-12-06 12:52:44 UTC) #17
zhaoqin
LGTM with nits. https://codereview.appspot.com/311350043/diff/60001/suite/tests/linux/sigaction_nosignals.c File suite/tests/linux/sigaction_nosignals.c (right): https://codereview.appspot.com/311350043/diff/60001/suite/tests/linux/sigaction_nosignals.c#newcode68 suite/tests/linux/sigaction_nosignals.c:68: #define SIGACT sizeof(struct kernel_sigaction_t) nit, would ...
7 years, 4 months ago (2016-12-08 15:01:29 UTC) #18
Edmund.Grimley.Evans
7 years, 4 months ago (2016-12-08 15:33:18 UTC) #19
Committed as
https://github.com/DynamoRIO/dynamorio/commit/0736badeb79d99c2b0b271f709eaa5a...

Final commit log: 
---------------
Add test linux.sigaction_nosignals, testing syscall without signals.

The "sigaction" system call is fairly complex, even in the absence of
signals, and should be emulated accurately by DynamoRIO. The test
is disabled as it does not yet pass.

Review-URL: https://codereview.appspot.com/311350043
---------------
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b