Skip to content

process bounds expressions for parameters with all the parameters available #7

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
dtarditi opened this issue Jun 28, 2016 · 1 comment
Closed

process bounds expressions for parameters with all the parameters available #7

dtarditi opened this issue Jun 28, 2016 · 1 comment
Assignees
@dtarditi
Milestone
Sprint 9

Comments

@dtarditi
Copy link
Member

Bounds expressions need to processed during clang's semantic checking/processing phase in a scope with all the parameters available. I am about to submit a pull request where they are being processed in a scope that contains only the parameters seen so far.

This is a little complicated to implement in clang's semantic checking/processing phase. You have to delay parsing of the bounds expressions because parsing and semantic processing are intermixed.
@dtarditi dtarditi mentioned this issue Jun 28, 2016
Merged
@dtarditi dtarditi added this to the Sprint 9 milestone Jun 28, 2016
@dtarditi dtarditi self-assigned this Jun 28, 2016
dtarditi added a commit that referenced this issue Jun 30, 2016
@dtarditi
Extend clang IR with bounds expressions and parse bounds expressions …
6d4d831 
…for parameters. (#8)

This change extends the clang IR to represent Checked C bounds expressions and optional bounds expressions for variable declarations. It also adds support for parsing bounds expressions and modifies parsing of function parameter lists to parse optional bounds expressions.

Bounds expressions are represented in the IR by adding a new abstract class BoundsExpr and subclassing it for count bounds expressions (count(e1) and byte_count(e1)), range bounds expressions (bounds(e1, e2)), and nullary bounds expressions(bounds(none)). AST printing, serialization, traversal, and tree transformations are extended handle the new expressions.

Bounds expressions are attached to variable declarations by adding an additional member to VarDecls. Many VarDecls will not have bounds expressions, so this adds extra space overhead to the representation of VarDecls. We can revisit this later if it becomes an issue.

To test the new bounds expressions, we add parsing of bounds expressions for function parameter lists and attach the parsed bounds expressions to the VarDecls for the parameters.

Bounds expressions for parameters need to be processed in a scope with all the parameters available. They are currently being processed in a scope that contains the parameters seen so far. This is a little complicated to implement in clang. You have to delay parsing of the bounds expressions. I will come back to this after getting basic parsing of bounds expressions working. I've opened issue #7 to track this.

Testing:
•This passes the current test baseline for this snapshot of clang:
•Wrote new feature tests of parsing of parameters with bounds declarations. There will be a separate pull request to the Github CheckedC repo for these tests.
•Passes the existing Checked C tests.
  Expected Passes    : 8942
  Expected Failures  : 21
  Unsupported Tests  : 206
  Unexpected Failures: 3

•We still need to test AST printing, serialization, traversal, and tree transformations. I've opened issues #4 , #3, #5, and #6 to track this.
dtarditi added a commit to dtarditi/checkedc-clang that referenced this issue Jul 8, 2016
@dtarditi
Parse bounds expression in function parameter list scope.
9859f07 
This change addresses issue checkedc#7 (process bounds expressions for parameters
with all the parameters available).  Currently, bounds expressions for
parameters are being parsed and type checked in the scope of the parameters
seen so far in a parameter list.  They need to be typechecked in a scope with
all the parameters for the parameter list.

To do this, parsing and type checking of bounds expressios for parameters
is deferred until the entire parameter list has been seen.  This is done
in two steps.  During the initial parse, the tokens for a superset of bounds
expressions are parsed and stored in a list.  They are then re-parsed using
ParseBoundsExpressions after all parameters have been seen.

This change also improve serror messages when a bounds expression is parsed
with misspelled contextual keyword.  For example, int x : boounds(x, x + 5).
Before, the code would simply give up after parsing the misspelled keyword
and the '('.  This led to additional error messages when the code tried to
 match the closing ')'.  This fix is to skip all the tokens up to but not
including the closing ')'.

Testing:
* Passes existing clang test suite.
* Added additional tests for parsing of function parameter lists with bounds
  expressions.  This will be commited separtely to the checked repo.
@dtarditi dtarditi mentioned this issue Jul 8, 2016
Merged
dtarditi added a commit that referenced this issue Jul 12, 2016
@dtarditi
Parse bounds expressions in function parameter list scopes. (#17)
4ca4bb4 
This change addresses issue #7 (process bounds expressions for parameters
 with all the parameters available). Currently, bounds expressions for
 parameters are being parsed and type checked in the scope of the parameters
 seen so far in a parameter list. They need to be typechecked in a scope with
 all the parameters for the parameter list.

To do this, parsing and type checking of bounds expressions for parameters
 is deferred until the entire parameter list has been seen. This is done
 in two steps. During the initial parse, the tokens for a superset of bounds
 expressions are parsed and stored in a list. They are then re-parsed using
 ParseBoundsExpression after all parameters have been seen.

This change also improves error messages when a bounds expression is parsed
 with misspelled contextual keyword. For example, int x : boounds(x, x + 5).
 Before, the code would simply give up after parsing the misspelled keyword
 and the '('. This led to additional error messages when the code tried to
 match the closing ')'. This fix is to skip all the tokens up to but not
 including the closing ')'.

Testing:
•Passes existing clang test suite.
•Added additional tests for parsing of function parameter lists with bounds expressions. This will be committed separately to the Checked C repo.
@dtarditi
Copy link
Member Author

This work is complete.

@AnnaKornfeldSimpson AnnaKornfeldSimpson mentioned this issue Jul 13, 2018
Closed
kkjeer pushed a commit that referenced this issue Sep 23, 2020
@DimitryAndric @zmodem
Instantiate Error in Target::GetEntryPointAddress() only when necessary
6406b6f 
When `Target::GetEntryPointAddress()` calls `exe_module->GetObjectFile()->GetEntryPointAddress()`, and the returned
`entry_addr` is valid, it can immediately be returned.

However, just before that, an `llvm::Error` value has been setup, but in this case it is not consumed before returning, like is done further below in the function.

In https://bugs.freebsd.org/248745 we got a bug report for this, where a very simple test case aborts and dumps core:

```
* thread #1, name = 'testcase', stop reason = breakpoint 1.1
    frame #0: 0x00000000002018d4 testcase`main(argc=1, argv=0x00007fffffffea18) at testcase.c:3:5
   1	int main(int argc, char *argv[])
   2	{
-> 3	    return 0;
   4	}
(lldb) p argc
Program aborted due to an unhandled Error:
Error value was Success. (Note: Success values must still be checked prior to being destroyed).

Thread 1 received signal SIGABRT, Aborted.
thr_kill () at thr_kill.S:3
3	thr_kill.S: No such file or directory.
(gdb) bt
#0  thr_kill () at thr_kill.S:3
#1  0x00000008049a0004 in __raise (s=6) at /usr/src/lib/libc/gen/raise.c:52
#2  0x0000000804916229 in abort () at /usr/src/lib/libc/stdlib/abort.c:67
#3  0x000000000451b5f5 in fatalUncheckedError () at /usr/src/contrib/llvm-project/llvm/lib/Support/Error.cpp:112
#4  0x00000000019cf008 in GetEntryPointAddress () at /usr/src/contrib/llvm-project/llvm/include/llvm/Support/Error.h:267
#5  0x0000000001bccbd8 in ConstructorSetup () at /usr/src/contrib/llvm-project/lldb/source/Target/ThreadPlanCallFunction.cpp:67
#6  0x0000000001bcd2c0 in ThreadPlanCallFunction () at /usr/src/contrib/llvm-project/lldb/source/Target/ThreadPlanCallFunction.cpp:114
#7  0x00000000020076d4 in InferiorCallMmap () at /usr/src/contrib/llvm-project/lldb/source/Plugins/Process/Utility/InferiorCallPOSIX.cpp:97
#8  0x0000000001f4be33 in DoAllocateMemory () at /usr/src/contrib/llvm-project/lldb/source/Plugins/Process/FreeBSD/ProcessFreeBSD.cpp:604
#9  0x0000000001fe51b9 in AllocatePage () at /usr/src/contrib/llvm-project/lldb/source/Target/Memory.cpp:347
#10 0x0000000001fe5385 in AllocateMemory () at /usr/src/contrib/llvm-project/lldb/source/Target/Memory.cpp:383
#11 0x0000000001974da2 in AllocateMemory () at /usr/src/contrib/llvm-project/lldb/source/Target/Process.cpp:2301
#12 CanJIT () at /usr/src/contrib/llvm-project/lldb/source/Target/Process.cpp:2331
#13 0x0000000001a1bf3d in Evaluate () at /usr/src/contrib/llvm-project/lldb/source/Expression/UserExpression.cpp:190
#14 0x00000000019ce7a2 in EvaluateExpression () at /usr/src/contrib/llvm-project/lldb/source/Target/Target.cpp:2372
#15 0x0000000001ad784c in EvaluateExpression () at /usr/src/contrib/llvm-project/lldb/source/Commands/CommandObjectExpression.cpp:414
#16 0x0000000001ad86ae in DoExecute () at /usr/src/contrib/llvm-project/lldb/source/Commands/CommandObjectExpression.cpp:646
#17 0x0000000001a5e3ed in Execute () at /usr/src/contrib/llvm-project/lldb/source/Interpreter/CommandObject.cpp:1003
#18 0x0000000001a6c4a3 in HandleCommand () at /usr/src/contrib/llvm-project/lldb/source/Interpreter/CommandInterpreter.cpp:1762
#19 0x0000000001a6f98c in IOHandlerInputComplete () at /usr/src/contrib/llvm-project/lldb/source/Interpreter/CommandInterpreter.cpp:2760
#20 0x0000000001a90b08 in Run () at /usr/src/contrib/llvm-project/lldb/source/Core/IOHandler.cpp:548
#21 0x00000000019a6c6a in ExecuteIOHandlers () at /usr/src/contrib/llvm-project/lldb/source/Core/Debugger.cpp:903
#22 0x0000000001a70337 in RunCommandInterpreter () at /usr/src/contrib/llvm-project/lldb/source/Interpreter/CommandInterpreter.cpp:2946
#23 0x0000000001d9d812 in RunCommandInterpreter () at /usr/src/contrib/llvm-project/lldb/source/API/SBDebugger.cpp:1169
#24 0x0000000001918be8 in MainLoop () at /usr/src/contrib/llvm-project/lldb/tools/driver/Driver.cpp:675
#25 0x000000000191a114 in main () at /usr/src/contrib/llvm-project/lldb/tools/driver/Driver.cpp:890```

Fix the incorrect error catch by only instantiating an `Error` object if it is necessary.

Reviewed By: JDevlieghere

Differential Revision: https://reviews.llvm.org/D86355

(cherry picked from commit 1ce07cd)
dopelsunce pushed a commit to dopelsunce/checkedc-clang that referenced this issue Sep 28, 2020
@dtarditi
Merge pull request checkedc#7 from dtarditi/master
20d5ae9 
Allow comparison and subtraction of ptr types.
@sulekhark sulekhark mentioned this issue Apr 13, 2021
Closed
sulekhark pushed a commit that referenced this issue Jul 21, 2021
@yonghong-song @tstellar
BPF: Fix a bug in peephole TRUNC elimination optimization
6564e0c 
Andrei Matei reported a llvm11 core dump for his bpf program
   https://bugs.llvm.org/show_bug.cgi?id=48578
The core dump happens in LiveVariables analysis phase.
  #4 0x00007fce54356bb0 __restore_rt
  #5 0x00007fce4d51785e llvm::LiveVariables::HandleVirtRegUse(unsigned int,
      llvm::MachineBasicBlock*, llvm::MachineInstr&)
  #6 0x00007fce4d519abe llvm::LiveVariables::runOnInstr(llvm::MachineInstr&,
      llvm::SmallVectorImpl<unsigned int>&)
  #7 0x00007fce4d519ec6 llvm::LiveVariables::runOnBlock(llvm::MachineBasicBlock*, unsigned int)
  #8 0x00007fce4d51a4bf llvm::LiveVariables::runOnMachineFunction(llvm::MachineFunction&)
The bug can be reproduced with llvm12 and latest trunk as well.

Futher analysis shows that there is a bug in BPF peephole
TRUNC elimination optimization, which tries to remove
unnecessary TRUNC operations (a <<= 32; a >>= 32).
Specifically, the compiler did wrong transformation for the
following patterns:
   %1 = LDW ...
   %2 = SLL_ri %1, 32
   %3 = SRL_ri %2, 32
   ... %3 ...
   %4 = SRA_ri %2, 32
   ... %4 ...

The current transformation did not check how many uses of %2
and did transformation like
   %1 = LDW ...
   ... %1 ...
   %4 = SRL_ri %2, 32
   ... %4 ...
and pseudo register %2 is used by not defined and
caused LiveVariables analysis core dump.

To fix the issue, when traversing back from SRL_ri to SLL_ri,
check to ensure SLL_ri has only one use. Otherwise, don't
do transformation.

Differential Revision: https://reviews.llvm.org/D97792

(cherry picked from commit 51cdb78)
@secure-sw-dev-bot secure-sw-dev-bot mentioned this issue Jan 16, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@dtarditi dtarditi

Labels
None yet
Projects
None yet
Milestone
Sprint 9
Development

No branches or pull requests
1 participant
@dtarditi