Skip to content

Parse bounds declarations for variable declarations. #18

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.

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

Merged
merged 2 commits into from
Jul 15, 2016

Conversation

dtarditi
Copy link
Member

@dtarditi dtarditi commented Jul 14, 2016

This change adds parsing of variable declarations with bounds declarations (issue #13).
For each declarator in a declaration, the declarator is parsed and then the
optional Checked C bounds declaration is parsed. The bounds declaration is
parsed before the optional initializing expression for the declarator. Because
the declarator has already been parsed and added to the current scope, the
bounds expression can be eagerly parsed.

One surprise with clang was that placing declarators for a declaration
on multiple lines caused a parsing error in the initial implementation,
while having all the declarators on one line did not. I traced this back to
special case code that looks for typographical mistakes
at line endings by calling MightBeDeclarator and generating an
error if MightBeDeclarator is false. MightBeDeclarator returns true
for syntactic items that might start a declarator. It has special
case checks to make sure that an identifier is followed by something
that might also be part of a declarator. For Checked C, an identifier
that starts a declarator may be followed by ':' and a bounds expression,
so allow ':' when the language options include Checked C.

This change also improves error handling during the parsing of bounds
expressions.

  • When an error occurs after having parsed an identifier and a left parenthesis,
    always scan for the matching right parenthesis. The scan for the matching
    right parenthesis was only happening in one specific case, leading to
    hard-to-understand spurious parsing errors.
  • Make a best effort to continue if an error occurs while parsing a
    bounds expression of the form bounds '(' e1 ',' e2, ')'. clang does not
    differentiate during parsing of expressions between semantic errors and
    parsing failures. It is important to continue parsing so that a semantic
    error does not cause a cascade of parsing errors.

These problems were uncovered during testing of parsing of variable declarations
with bounds expressions. Specifically, using an incorrect bounds expression
in a variable declaration with an initializer caused a spurious parsing
errors.

Testing:

  • Created a new feature test for parsing of declarations with bounds
    (parsing_bounds_var_declarations.c). This will be committed separately to the
    checkedc repo.
  • Passes existing Checked C tests.
  • Passes existing clang base line tests.

This change adds  parsing of variable declarations with bounds declarations.
For each declaration in a declaration, the declarator is parsed and then the
optional Checked C bounds declaration  is parsed.  The bounds declaration is
parsed before the optional initializing expression.  Because the declarator
has already been parsed and add to the current scope, the bounds expression
can be eagerly parsed.

This change also improves error handling during the parsing of bounds
expressions.
- When an error occurs after having parsed an identifier and a left parenthesis,
  always scan for the matching right parenthesis.  The scan for the matching
  right parenthesis was only happening in one specific case, leading to
  hard-to-understand spurious parsing errors.
- Make a best effort to continue if an error occurs while parsing a
  bounds expression of the form bounds '(' e1 ',' e2, ')'.  clang does not
  differentiate during parsing of expressions between semantic errors and
  parsing failures.  It is important to continue parsing so that a semantic
  error does not cause a cascade of parsing errors.

These problems were uncovered during testing of parsing of variable declarations
with bounds expression.  Specifically, using an incorrect bounds expression
in a variable declaration with an initializer caused a spurious parsing
errors.

Testing:
- Created a new feature test for parsing of declarations with bounds
  (parsing_bounds_var_declarations.c).  This will be committed separately to the
  checkedc repo.
- Passes existing Checked C tests.
- Passes existing clang base line tests.
@msftclas
Copy link

Hi @dtarditi, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!


It looks like you're a Microsoft contributor (David Tarditi). If you're full-time, we DON'T require a Contribution License Agreement. If you are a vendor, please DO sign the electronic Contribution License Agreement. It will take 2 minutes and there's no faxing! https://cla.microsoft.com.

TTYL, MSBOT;

@@ -1623,7 +1623,8 @@ bool Parser::MightBeDeclarator(unsigned Context) {
// and in block scope it's probably a label. Inside a class definition,
// this is a bit-field.
return Context == Declarator::MemberContext ||
(getLangOpts().CPlusPlus && Context == Declarator::FileContext);
(getLangOpts().CPlusPlus && Context == Declarator::FileContext) ||
getLangOpts().CheckedC;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CheckedC [](start = 27, length = 8)

For posterity, it might be helpful to update the comment above to explain why there's a special-case here for Checked C. (I assume it's for the ':' bounds expr syntax for namespace-scope variable declarations.)

@reubeno
Copy link

reubeno commented Jul 15, 2016

Modulo my one comment comment :), looks good to me!

@dtarditi
Copy link
Member Author

Good idea - I've added a comment explaining the case for Checked C.

@dtarditi dtarditi merged commit 9b85c1c into checkedc:master Jul 15, 2016
@dtarditi dtarditi deleted the issue13 branch July 29, 2016 20:16
mgrang pushed a commit that referenced this pull request Sep 27, 2019
Cherry-picked from commit 2d090f18a30386e4bb2b8dbfc1e2558d77198f4b

    Fix the generation of llvm-lit.py so that llvm-lit can be used to run Checked C tests in the Checked C repo from the command line. Several arguments were missing from configure_lit_site_cfg in the CMakeLists.txt file for the checkedc-wrapper directory.  This fixes Checked C issue checkedc/checkedc#340.

    Also fix some warnings about missing tools when running the tests.  The tools are clang-specific test executables and tools not used by the Checked C repo tests.  Delete them from the list of required tools.
kkjeer pushed a commit that referenced this pull request Sep 23, 2020
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 pull request Sep 28, 2020
* In bounds declaration checking, handle bounds for parameter variables with
  array types specially.
* Add missing address-of cases for &*e and &e1[e2].

This addresses Github issues checkedc#18 and checkedc#19.
dopelsunce pushed a commit to dopelsunce/checkedc-clang that referenced this pull request Sep 28, 2020
…heckedc#42)

Add a description of alternative bounds choices for &*e.  This
addresses issue checkedc#18.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants