Skip to content
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

Segmentation fault in code involving generic functions #48323

Closed
nightscape opened this issue Feb 5, 2022 · 13 comments
Closed

Segmentation fault in code involving generic functions #48323

nightscape opened this issue Feb 5, 2022 · 13 comments
Labels
area-front-end Use area-front-end for front end / CFE / kernel format related issues. type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)

Comments

@nightscape
Copy link

Hi all,

I'm writing a library that allows to build a Directed Acyclic Graph of descriptions of Stream transformations which can then be applied to actual input streams.
One of the core classes is a StreamNode<T> from which a Stream<T> will be created, optionally allowing a StreamTransformer<T> to be retrieved and applied.
As the graph has different generic parameters T in StreamNodes, I need to go through some hoops to get the Dart compiler accept my code.
In my latest iteration, I thought I had made progress on the issue, but instead I get the Dart to crash with an error like this

===== CRASH =====
si_signo=Segmentation fault([11](https://github.com/nightscape/stream_graph.dart/runs/5080365009?check_suite_focus=true#step:6:11)), si_code=1, si_addr=0x4cf0039015f
version=2.16.0 (stable) (Mon Jan 31 15:28:59 2022 +0100) on "linux_x64"
pid=1690, thread=1728, isolate_group=main(0x55a6da5b6800), isolate=main(0x55a6dc[14](https://github.com/nightscape/stream_graph.dart/runs/5080365009?check_suite_focus=true#step:6:14)e000)
isolate_instructions=55a6d7f92540, vm_instructions=55a6d7f92540
  pc 0x000055a6d80bd7b4 fp 0x00007fd3734fd9c0 dart::BootstrapNatives::DN_Internal_boundsCheckForPartialInstantiation(dart::Thread*, dart::Zone*, dart::NativeArguments*)+0x44
  pc 0x000055a6d8[17](https://github.com/nightscape/stream_graph.dart/runs/5080365009?check_suite_focus=true#step:6:17)0924 fp 0x00007fd3734fda40 dart::NativeEntry::BootstrapNativeCallWrapper(_Dart_NativeArguments*, void (*)(_Dart_NativeArguments*))+0xb4
  pc 0x00007fd377502654 fp 0x00007fd3734fda80 Unknown symbol

The full stack trace can be seen here:
https://github.com/nightscape/stream_graph.dart/runs/5080365009?check_suite_focus=true

I can reproduce this locally on a Mac ARM64 and in an AMD64 Linux env in the Github actions.
Is this something worth looking into, or shall I just go looking for a solution that doesn't make Dart crash?

@nightscape nightscape added the bug label Feb 5, 2022
@leafpetersen leafpetersen transferred this issue from dart-lang/language Feb 5, 2022
@leafpetersen leafpetersen added the area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. label Feb 5, 2022
@leafpetersen
Copy link
Member

Transferring to the SDK repo. cc @a-siva

@a-siva
Copy link
Contributor

a-siva commented Feb 7, 2022

@nightscape thanks for filing the issue, is it is possible for us to get instructions on how we can get a local reproduction of the crash

@srawlins srawlins added type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) and removed bug labels Feb 24, 2022
@a-siva a-siva added the needs-info We need additional information from the issue author (auto-closed after 14 days if no response) label Feb 25, 2022
@a-siva
Copy link
Contributor

a-siva commented Feb 25, 2022

@nightscape any possibilities of getting a setup for reproduction

@nightscape
Copy link
Author

@a-siva Sorry for taking so long to respond, this slipped under the radar.
The commands to reproduce would be

git clone https://github.com/nightscape/stream_graph.dart.git
cd ./stream_graph.dart/
git checkout a896451677aeb55eeb73bd5959534df8add27164
dart test

@a-siva
Copy link
Contributor

a-siva commented Apr 25, 2022

Thanks for the repro.
I ran a debug version of dart and the crash backtrace is as follows

00:02 +0: loading test/stream_graph_test.dart                                   ../../runtime/vm/object.cc: 3366: error: expected: !sup_type_arg.IsNull()
version=2.18.0-edge.e9e48993f8056f2a8879adc01901cf81ba7fae6f (be) (Mon Apr 25 21:47:14 2022 +0000) on "linux_x64"
pid=4158689, thread=4158713, isolate_group=main(0x55ac53e61000), isolate=main(0x55ac557e4000)
isolate_instructions=55ac51239f20, vm_instructions=55ac51239f20
  pc 0x000055ac51642b4c fp 0x00007f89cebfdda0 dart::Profiler::DumpStackTrace(void*)+0x7c
  pc 0x000055ac5123a0d4 fp 0x00007f89cebfde80 dart::Assert::Fail(char const*, ...) const+0x84
  pc 0x000055ac51558edc fp 0x00007f89cebfdee0 dart::Class::ComputeNumTypeArguments() const+0x37c
  pc 0x000055ac51558f70 fp 0x00007f89cebfdf10 dart::Class::NumTypeArguments() const+0x50
  pc 0x000055ac51479c95 fp 0x00007f89cebfdf70 dart::ClassFinalizer::FinalizeType(dart::AbstractType const&, dart::ClassFinalizer::FinalizationKind, dart::ZoneGrowableHandlePtrArray<dart::AbstractType const>*)+0x255
  pc 0x000055ac514797ae fp 0x00007f89cebfdff0 dart::ClassFinalizer::ExpandAndFinalizeTypeArguments(dart::Zone*, dart::AbstractType const&, dart::ZoneGrowableHandlePtrArray<dart::AbstractType const>*)+0x25e
  pc 0x000055ac51479eef fp 0x00007f89cebfe050 dart::ClassFinalizer::FinalizeType(dart::AbstractType const&, dart::ClassFinalizer::FinalizationKind, dart::ZoneGrowableHandlePtrArray<dart::AbstractType const>*)+0x4af
  pc 0x000055ac514774e0 fp 0x00007f89cebfe100 dart::ClassFinalizer::FinalizeTypesInClass(dart::Class const&)+0x240
  pc 0x000055ac514770c2 fp 0x00007f89cebfe2b0 dart::ClassFinalizer::ProcessPendingClasses()+0x252
  pc 0x000055ac514fe32a fp 0x00007f89cebfe810 dart::kernel::KernelLoader::LoadEntireProgram(dart::kernel::Program*, bool)+0x79a


@a-siva
Copy link
Contributor

a-siva commented Apr 25, 2022

//cc @alexmarkov

@alexmarkov alexmarkov self-assigned this Apr 26, 2022
@alexmarkov
Copy link
Contributor

There are 2 bugs triggered on this code:

  1. Assertion failure in DEBUG mode during finalization. It is caused by an incorrect assertion and unlikely to affect users.

Small repro:

abstract class GraphNode extends Comparable<dynamic> {
  int compareTo(dynamic other) => 0;
}

abstract class StreamNode<T> extends GraphNode {}

class TransformNode<S, T> extends StreamNode<T> {}

main() {
  print(TransformNode());
}
  1. Crash around _boundsCheckForPartialInstantiation.

Small repro:

void Function<T>()? genericFunction;

void foo(void Function()? arg) {
  print(arg);
}

void main() {
  foo(genericFunction);
}

The problem is that CFE creates Instantiation node which takes a nullable value, but VM doesn't expect to see null as an argument to that node (it expects to see a generic closure which should be instantiated using the Instantiation.typeArguments). The VM implementation of that node didn't expect null since it was added in https://dart-review.googlesource.com/c/sdk/+/34400/.

@johnniwinther Could you clarify if this is a valid AST and VM should support this case?
@leafpetersen What would be a correct behavior in this case? A compile-time error, run-time error or silently "instantiating" null?

copybara-service bot pushed a commit that referenced this issue Apr 27, 2022
TEST=vm/dart/regress_48323_1_test

Issue: #48323
Change-Id: I50c04a269183c0df5b4ce0ce7be32cd2385e95d8
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/242601
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Commit-Queue: Alexander Markov <alexmarkov@google.com>
@leafpetersen
Copy link
Member

This is related to this issue.

I would have expected this code to be a static error (just as implicitly tearing off a call method from a nullable value is a static error), but it seems that neither the analyzer nor the cfe emits an error there.

We could make the breaking change to make this an error, or we could specify these to be null aware (in which case the behavior would be essentially to produce null if the value was null, and otherwise produce the generic instantiation. Either way it seems like we should be consistent. cc @munificent @eernstg @lrhn

@eernstg
Copy link
Member

eernstg commented Apr 28, 2022

I could not find any rules about generic function instantiation in the nnbd feature spec. The upcoming update to the language specification where null safety is integrated specifies that generic function instantiation can only take place when the function being instantiated has a function type, not a type of the form T? for any T. So we should get a compile-time error according to the ongoing specification work.

However, it would seem reasonable to generalize generic function instantiation to be null-aware if we do the same thing for call insertion, cf. dart-lang/language#1333.

@johnniwinther
Copy link
Member

@alexmarkov The CFE shouldn't generate instantiation on a nullable expressions. It should either be an error or converted into genericFunction != null ? genericFunction<dynamic> : null.

@alexmarkov
Copy link
Contributor

Debug assertion is fixed in 609ba96.

@johnniwinther Could you take a stab at fixing the 2nd bug in the CFE?

@alexmarkov alexmarkov removed their assignment Apr 28, 2022
@alexmarkov alexmarkov added area-front-end Use area-front-end for front end / CFE / kernel format related issues. and removed area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. needs-info We need additional information from the issue author (auto-closed after 14 days if no response) labels Apr 28, 2022
@johnniwinther
Copy link
Member

Fix for the CFE in progress: https://dart-review.googlesource.com/c/sdk/+/242869

copybara-service bot pushed a commit that referenced this issue May 2, 2022
This might be a compile-time error but it should at least not create
an Instantiation on a potentially nullable expression.

In response to #48323

Change-Id: I6682ddbb8db82623ac7bf4ecf37f8535377f4ca0
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/242869
Reviewed-by: Chloe Stefantsova <cstefantsova@google.com>
Commit-Queue: Johnni Winther <johnniwinther@google.com>
@mkustermann
Copy link
Member

I believe this issue was fixed by @johnniwinther in 7ab8930. Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-front-end Use area-front-end for front end / CFE / kernel format related issues. type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)
Projects
None yet
Development

No branches or pull requests

8 participants