Skip to content

Comments

Bugfix: Arduino Data Dir nil dereference#188

Closed
Kraiwin-Namsri wants to merge 2 commits intoarduino:mainfrom
Kraiwin-Namsri:main
Closed

Bugfix: Arduino Data Dir nil dereference#188
Kraiwin-Namsri wants to merge 2 commits intoarduino:mainfrom
Kraiwin-Namsri:main

Conversation

@Kraiwin-Namsri
Copy link

@Kraiwin-Namsri Kraiwin-Namsri commented Jun 13, 2024

Bug

func New(path ...string) *Path {
	if len(path) == 0 {
		return nil
	}
	if len(path) == 1 && path[0] == "" {
		return nil
	}
	res := &Path{path: path[0]}
	if len(path) > 1 {
		return res.Join(path[1:]...)
	}
	return res
}
dataDirPath := paths.New(dataDir)
return dataDirPath.Canonical(), nil

When dataDir is an empty string, paths.New(dataDir) return nil, later de-referencing nil.

Logs:

[ERROR][2024-06-13 18:04:12] .../vim/lsp/rpc.lua:734	"rpc"	"arduino-language-server"	"stderr"	"18:04:12.708708 \27[96mINIT --- : Arduino Data Dir -> \27[0m\n"
[ERROR][2024-06-13 18:04:12] .../vim/lsp/rpc.lua:734	"rpc"	"arduino-language-server"	"stderr"	"18:04:12.708773 Panic: runtime error: invalid memory address or nil pointer dereference\n\ngoroutine 34 [running]:\nruntime/debug.Stack()\n\t/usr/local/go/src/runtime/debug/stack.go:24 +0x65\ngithub.com/arduino/arduino-language-server/streams.CatchAndLogPanic()\n\t/home/build/streams/panics.go:29 +0x74\npanic({0xa725a0, 0x1066130})\n\t/usr/local/go/src/runtime/panic.go:844 +0x258\ngithub.com/arduino/go-paths-helper.(*Path).Clone(...)\n\t/go/pkg/mod/github.com/arduino/go-paths-helper@v1.7.0/paths.go:81\ngithub.com/arduino/go-paths-helper.(*Path).Canonical(0xc0002d1b78?)\n\t/go/pkg/mod/github.com/arduino/go-paths-helper@v1.7.0/paths.go:536 +0x1e\ngithub.com/arduino/arduino-language-server/ls.(*INOLanguageServer).extractDataFolderFromArduinoCLI(0xc0001e2300, {0xc45e20, 0xc00010e018})\n\t/home/build/ls/ls.go:1485 +0xa1f\ngithub.com/arduino/arduino-language-server/ls.(*INOLanguageServer).initializeReqFromIDE.func1()\n\t/home/build/ls/ls.go:215 +0x369\ncreated by github.com/arduino/arduino-language-server/ls.(*INOLanguageServer).initializeReqFromIDE\n\t/home/build/ls/ls.go:189 +0x20a\n\n18:04:12.708813 \27[92m                 textDocument/didOpen: \27[93mlocked (waiting clangd)\27[0m\27[0m\n18:04:12.708825 \27[92m                 textDocument/didOpen: clangd startup failed: quitting Language server\27[0m\n"

Steps to reproduce

This needs work, as I am not entirely sure why this bug occurs on my system.
It seems like arduino-cli.yaml does not contain the dataFolder. Or at least arduino-cli --config-file <file> config dump --format json does not return the dataFolder.

Extra information

I am using lvim together with mason. Mason installed arduino-language-server 0.7.6. Above logs describe the program.

Please feel free to leave a comment on how to improve this PR!

Kind regards,
Kraiwin

@Kraiwin-Namsri
Bugfix: Arduino Data Dir nil dereference
84ff525 
Bug:
	When dataDir is an empty string: paths.New() returns nil.
	next Canonical() get's called dereferencing nil.
Fix:
	Ignore
@CLAassistant
Copy link

CLAassistant commented Jun 13, 2024 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@alessio-perugini
Copy link
Contributor

alessio-perugini commented Jun 14, 2024

@Kraiwin-Namsri Hello what version of the arduino-cli are you running?

@Kraiwin-Namsri
Copy link
Author

Kraiwin-Namsri commented Jun 14, 2024

arduino-cli Version: 1.0.0 Commit: 05c9852a Date: 2024-06-12T14:13:32Z

@Kraiwin-Namsri
Copy link
Author

Kraiwin-Namsri commented Jun 14, 2024 •
edited
Loading

The version of arduino-cli indeed seems to be the problem. When running arduino-cli config dump --format json

Output of 1.0.0:

{
  "config": {
    "board_manager": {
      "additional_urls": [
        "http://arduino.esp8266.com/stable/package_esp8266com_index.json"
      ]
    }
  }
}

Following the output of 0.35.3:

{
  "board_manager": {
    "additional_urls": [
      "http://arduino.esp8266.com/stable/package_esp8266com_index.json"
    ]
  },
  "build_cache": {
    "compilations_before_purge": 10,
    "ttl": 2592000000000000
  },
  "daemon": {
    "port": "50051"
  },
  "directories": {
    "data": "/home/winus/.arduino15",
    "downloads": "/home/winus/.arduino15/staging",
    "user": "/home/winus/Arduino"
  },
  "library": {
    "enable_unsafe_install": false
  },
  "logging": {
    "file": "",
    "format": "text",
    "level": "info"
  },
  "metrics": {
    "addr": ":9090",
    "enabled": true
  },
  "output": {
    "no_color": false
  },
  "sketch": {
    "always_export_binaries": false
  },
  "updater": {
    "enable_notification": true
  }
}

@alessio-perugini
Copy link
Contributor

alessio-perugini commented Jun 14, 2024 •
edited
Loading

@Kraiwin-Namsri Thank you for your valuable feedback. It seems to be a problem with the arduino-cli 1.0.0. I'm escalating this internally.
For now, you can fallback on using the arduino-cli 0.53.3 or directly set that property in the arduino-cli config file. I'll come back to you as soon as we have released the fix 🙏

@alessio-perugini alessio-perugini mentioned this pull request Jun 14, 2024
Closed
3 tasks
@cmaglie cmaglie mentioned this pull request Jun 14, 2024
Merged
3 tasks
@cmaglie
Copy link
Member

cmaglie commented Jun 14, 2024

I've pushed a possible fix in #189. @Kraiwin-Namsri may you test it?

@per1234 per1234 added topic: code Related to content of the project itself type: imperfection Perceived defect in any part of project labels Jun 14, 2024
@cmaglie
Copy link
Member

cmaglie commented Jun 14, 2024 •
edited by per1234
Loading

Superseded by #189

@cmaglie cmaglie closed this Jun 14, 2024
@per1234 per1234 added the conclusion: duplicate Has already been submitted label Jun 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

conclusion: duplicate Has already been submitted topic: code Related to content of the project itself type: imperfection Perceived defect in any part of project

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@Kraiwin-Namsri @CLAassistant @alessio-perugini @cmaglie @per1234