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

meson introspect can hang in some cases #11763

Open
kcgen opened this issue May 3, 2023 · 13 comments
Open

meson introspect can hang in some cases #11763

kcgen opened this issue May 3, 2023 · 13 comments

Comments

@kcgen
Copy link

kcgen commented May 3, 2023

Describe the bug
When fed DOSBox Staging's meson.build file, meson introspect hangs.

To Reproduce

cd /tmp
git clone --depth 1 https://github.com/dosbox-staging/dosbox-staging
cd dosbox-staging
meson introspect --projectinfo meson.build

Expected behavior
Printing of the project information to stdout, in JSON format.

system parameters

  • cross build? : no
  • operating system: Confirmed on Arch (latest), Ubuntu 22.04 and 23.04
  • Python version: 3.11.2
  • meson --version: 1.1.0

Thanks to @GranMinigun for flagging this via IRC.

@bruchar1
Copy link
Member

bruchar1 commented May 3, 2023

The culprit is libsdlcd_sources = files(libsdlcd_sources) in src\libs\sdlcd\meson.build:20 .

Minimal example to reproduce:

project('sample', 'c')

f = ['sample.c']
f = files(f)

exe = executable('sample', f, install : true)

@bruchar1
Copy link
Member

bruchar1 commented May 3, 2023

Similar problem occurs when performing a plusassignment. In ast/interpreter.py, assignment and evaluate_plusassign functions both replace the current assigned value by the new value, causing previous value to be lost. In the case of a +=, resulting introspection data would only preserve the added values of the assignment.

@bruchar1
Copy link
Member

bruchar1 commented May 3, 2023

For example, meson introspect --targets meson.build on this file:

project('sample', 'c')

f = ['sample1.c']
f += ['sample2.c']

exe = executable('sample', f)

results in

[{"name": "sample", "id": "sample@exe", "type": "executable", "defined_in": "meson.build", "filename": ["sample.exe"], "build_by_default": true, "target_sources": [{"language": "unknown", "compiler": [], "parameters": [], "sources": ["C:\\env\\meson\\sample\\sample2.c"], "generated_sources": []}], "depends": [], "extra_files": [], "subproject": null, "installed": false}]

sample1.c is missing from introspection data.

@bruchar1
Copy link
Member

bruchar1 commented May 3, 2023

Incidentally, the rewriter will work, because it will only alter the last assignment, but it can result in a duplicated source file because it doesn't know about previous assignments, and it will hang in an infinite loop with the first example.

@Volker-Weissmann
Copy link
Contributor

Describe the bug When fed DOSBox Staging's meson.build file, meson introspect hangs.

To Reproduce

cd /tmp
git clone --depth 1 https://github.com/dosbox-staging/dosbox-staging
cd dosbox-staging
meson introspect --projectinfo meson.build

Expected behavior Printing of the project information to stdout, in JSON format.

system parameters

* cross build? : no

* operating system: Confirmed on Arch (latest), Ubuntu 22.04 and 23.04

* Python version: 3.11.2

* `meson --version`: 1.1.0

Thanks to @GranMinigun for flagging this via IRC.

If you replace meson.build with the path to a build directory it works:

meson introspect --projectinfo builddir

@GranMinigun
Copy link

This is an issue with IDEs like KDevelop and Qt Creator which do it that way.

@kcgen
Copy link
Author

kcgen commented May 14, 2023

If you replace meson.build with the path to a build directory it works

@Volker-Weissmann , indeed, that's a workaround to side-step this bug.

The ticket's purpose is specifically to point out the bug exactly as specified in the reproduction steps to help the developers reproduce and fix it (not to work-around it).

@eli-schwartz
Copy link
Member

Introspecting a build directory doesn't provide AST information about the meson.build files, so it isn't a workaround so much as doing something else entirely.

...

Minimal example to reproduce:

project('sample', 'c')

f = ['sample.c']
f = files(f)

exe = executable('sample', f, install : true)

poking around a bit:

while inqueue:
curr = inqueue.pop(0)
arg_node = None
assert isinstance(curr, BaseNode)
if isinstance(curr, FunctionNode):
arg_node = curr.args
elif isinstance(curr, ArrayNode):
arg_node = curr.args
elif isinstance(curr, IdNode):
# Try to resolve the ID and append the node to the queue
assert isinstance(curr.value, str)
var_name = curr.value
if var_name in self.assignments:
tmp_node = self.assignments[var_name]
if isinstance(tmp_node, (ArrayNode, IdNode, FunctionNode)):
inqueue += [tmp_node]
elif isinstance(curr, ArithmeticNode):
inqueue += [curr.left, curr.right]
if arg_node is None:
continue
arg_nodes = arg_node.arguments.copy()
# Pop the first element if the function is a build target function
if isinstance(curr, FunctionNode) and curr.func_name in BUILD_TARGET_FUNCTIONS:
arg_nodes.pop(0)
elementary_nodes = [x for x in arg_nodes if isinstance(x, (str, StringNode))]
inqueue += [x for x in arg_nodes if isinstance(x, (FunctionNode, ArrayNode, IdNode, ArithmeticNode))]
if elementary_nodes:
res += [curr]

files() is not an elementary node, so the value of f is appended to inqueue:

# third value of f, first non-assignment reference
IdNode(lineno=4, colno=10, filename='meson.build', end_lineno=4, end_colno=10)

Since this is an IdNode, we look up self.assignments[curr.value], and get:

FunctionNode(lineno=4, colno=4, filename='meson.build', end_lineno=4, end_colno=12)

This is where we get stuck, because the FunctionNode is pointing at the second assignment to f = , and has args.arguments with a value of the IdNode that we mentioned before... which is fair in context, but when we loop back around we look up the latest value of f via self.assignments.

Not sure what the good solution here is. One option might be to hoist some code out of resolve_node, which already has a loop detector.

@Volker-Weissmann
Copy link
Contributor

To avoid duplicate work, please be informed that I'm currently working on it, fix will (hopefully) come this week.

@eli-schwartz
Copy link
Member

Awesome, I wasn't working on it at all beyond the analysis I documented above.

@Volker-Weissmann
Copy link
Contributor

Volker-Weissmann commented May 21, 2023

While fixing this bug I found quite a lot of limitations/bugs in the meson rewriter. I think I'll rewrite some major parts of the rewriter.

Volker-Weissmann added a commit to Volker-Weissmann/meson that referenced this issue Sep 20, 2023
…tool

The rewriter and the static introspection tool
used to be very broken, now it is *less* broken.

Fixes mesonbuild#11763
Volker-Weissmann added a commit to Volker-Weissmann/meson that referenced this issue Sep 20, 2023
…tool

The rewriter and the static introspection tool
used to be very broken, now it is *less* broken.

Fixes mesonbuild#11763
Volker-Weissmann added a commit to Volker-Weissmann/meson that referenced this issue Sep 20, 2023
…tool

The rewriter and the static introspection tool
used to be very broken, now it is *less* broken.

Fixes mesonbuild#11763
Volker-Weissmann added a commit to Volker-Weissmann/meson that referenced this issue Sep 22, 2023
The rewriter and the static introspection tool
used to be very broken, now it is *less* broken.

The most important changes are:

1. We now have class UnknownValue for more explicit handling
	of situations that are too complex/impossible.

2. If you write
	```
	var = 'foo'
	name = var
	var = 'bar'
	executable(name, 'foo.c')
	```
	the tool now knows that the name of the executable is foo and not bar.
	See dataflow_dag and node_to_runtime_value for details on how we do this.

Fixes mesonbuild#11763
Volker-Weissmann added a commit to Volker-Weissmann/meson that referenced this issue Oct 2, 2023
The rewriter and the static introspection tool
used to be very broken, now it is *less* broken.

The most important changes are:

1. We now have class UnknownValue for more explicit handling
	of situations that are too complex/impossible.

2. If you write
	```
	var = 'foo'
	name = var
	var = 'bar'
	executable(name, 'foo.c')
	```
	the tool now knows that the name of the executable is foo and not bar.
	See dataflow_dag and node_to_runtime_value for details on how we do this.

Fixes mesonbuild#11763
Volker-Weissmann added a commit to Volker-Weissmann/meson that referenced this issue Oct 2, 2023
The rewriter and the static introspection tool
used to be very broken, now it is *less* broken.

The most important changes are:

1. We now have class UnknownValue for more explicit handling
	of situations that are too complex/impossible.

2. If you write
	```
	var = 'foo'
	name = var
	var = 'bar'
	executable(name, 'foo.c')
	```
	the tool now knows that the name of the executable is foo and not bar.
	See dataflow_dag and node_to_runtime_value for details on how we do this.

Fixes mesonbuild#11763
@bruchar1
Copy link
Member

@Volker-Weissmann did you finally fixed this bug?

@Volker-Weissmann
Copy link
Contributor

@Volker-Weissmann did you finally fixed this bug?

Yes: It is waiting for review, then I will squash the commits and we can merge.

#12277

Volker-Weissmann added a commit to Volker-Weissmann/meson that referenced this issue Oct 30, 2023
The rewriter and the static introspection tool
used to be very broken, now it is *less* broken.

The most important changes are:

1. We now have class UnknownValue for more explicit handling
	of situations that are too complex/impossible.

2. If you write
	```
	var = 'foo'
	name = var
	var = 'bar'
	executable(name, 'foo.c')
	```
	the tool now knows that the name of the executable is foo and not bar.
	See dataflow_dag and node_to_runtime_value for details on how we do this.

Fixes mesonbuild#11763
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants