Skip to content

[Compiler] Fix NullReferenceException in SemanticModel queries on compound assignments - #2096

Merged
RobertvanderHulst merged 1 commit into
X-Sharp:devfrom
hpetriffer:fix/semanticmodel-discarded-diagnostics-nre
Oct 6, 2026
Merged

RobertvanderHulst merged 1 commit into
X-Sharp:devfrom
hpetriffer:fix/semanticmodel-discarded-diagnostics-nre

Conversation

@hpetriffer

Copy link
Copy Markdown
Contributor

Two X#-specific blocks in the binder enumerate diagnostics.DiagnosticBag without a null check. When a public SemanticModel query binds a member, diagnostics is BindingDiagnosticBag.Discarded, whose DiagnosticBag is null.

  • Binder_Operators.cs, BindCompoundAssignment: the "clear 9020" block throws. Result: GetSymbolInfo / GetTypeInfo throw NullReferenceException for the nodes of any statement that contains a compound assignment (+=, -=, ...). Measured downstream in a tool that queries the semantic model of a large VO solution: 36,421 failed semantic-model queries before, 94 after.
  • Binder_Expressions.cs, CheckVOIndexedValue: foreach (... in diagnostics.DiagnosticBag?.AsEnumerable()) enumerates null. Same pattern; not seen in practice, fixed for consistency (hardening).

Both guards use diagnostics.AccumulatesDiagnostics, the idiom already used in Binder_Operators.cs. The guarded code only filters diagnostics. With a real bag (every xsc compile) the behavior is unchanged. In CheckVOIndexedValue only the diagnostic filter is guarded; the CheckValue binding result above it still runs.

xsc output is not affected. Analyzers, IDE features and other tools that use the SemanticModel API are.

No existing issue found (searched for SemanticModel NullReferenceException, Discarded DiagnosticBag, BindCompoundAssignment).

Repro

FUNCTION Test() AS INT
    LOCAL n AS INT
    n := 1
    n += 2
    RETURN n
using LanguageService.CodeAnalysis;
using LanguageService.CodeAnalysis.XSharp;

var parsed = XSharpCommandLineParser.Default.Parse(new[] { "/dialect:Core", "/t:library", "test.prg" }, dir, null);
var tree   = XSharpSyntaxTree.ParseText(source, (XSharpParseOptions)parsed.ParseOptions, "test.prg");
var comp   = XSharpCompilation.Create("t", new[] { tree }, refs, (XSharpCompilationOptions)parsed.CompilationOptions);
var model  = comp.GetSemanticModel(tree);
foreach (var node in tree.GetRoot().DescendantNodes())
{
    model.GetSymbolInfo(node);   // before: NullReferenceException in Binder.BindCompoundAssignment
    model.GetTypeInfo(node);
}

There is no maintained test project that drives the public SemanticModel API (XSTestCodeAnalysis is not part of any solution), so the repro is included here instead of as a unit test.

Testing

Based on dev @ ae0ed9fdb3. Release build via src\buildcompiler.cmd Release, tests via RunCompilertests.cmd.

before after
Build 0 warnings, 0 errors 0 warnings, 0 errors
Repro, Core dialect 3 of 23 nodes throw 0 throw
Repro, VO dialect 3 of 27 nodes throw 0 throw
Compiler tests (fixed) 991 / 1029 pass 991 / 1029 pass
Compiler tests (broken) 120 / 120 as expected 120 / 120 as expected

The 38 failing fixed tests fail on dev already; the set of failing test IDs is identical before and after.

🤖 Generated with Claude Code

…pound assignments

Two X#-specific blocks in the binder enumerate diagnostics.DiagnosticBag
without a null check. When a public SemanticModel query binds, diagnostics
is BindingDiagnosticBag.Discarded, whose DiagnosticBag is null.

- Binder_Operators.cs, BindCompoundAssignment: the "clear 9020" block threw
  for every compound assignment (+=, -=, ...).
- Binder_Expressions.cs, CheckVOIndexedValue: foreach over
  DiagnosticBag?.AsEnumerable() enumerated null (hardening, not observed).

Both guards use diagnostics.AccumulatesDiagnostics. The guarded code only
filters diagnostics, so xsc output is unchanged. In CheckVOIndexedValue the
CheckValue binding result still runs; only the diagnostic filter is guarded.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@RobertvanderHulst
RobertvanderHulst merged commit 239fcdf into X-Sharp:dev Oct 6, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants