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

Fix: GLIBCXX version detection bug in check-requirements-linux.sh (issue #204186) #204635

Merged
merged 10 commits into from Feb 8, 2024

Conversation

NorthSecond
Copy link
Contributor

  • Existing detection scripts simply use the awk command to record the version number in the libstdc++.so filename,
  • but in some specific versions of glibc++, the detailed version number is not indicated in the filename.
    • Like the issue Dev Containers not Working on Ubuntu 22.04.3 LTS #204186 , the OS version (Ubuntu 22.04) supports GLIBCXX_3.4.25 but fails version checking because the filename is simply libstdc++.so.6,
    • and we have reproduced this issue when connecting to the Ubuntu 22.04 server using ssh.
  • Instead of simply using the awk command to filter file names, we use a script to simulate the strings /usr/lib/x86_64-linux-gnu/libstdc++.so.6 | grep GLIBCXX operation, which reads the highest-supported version number of GLIBCXX, and determines whether the existing environment supports the minimum VSCode requirements. instead of simply filtering filenames with the awk command.

- Existing detection scripts simply use the `awk` command to record the version number in the `libstdc++.so` filename,
- but in some specific versions of glibc++, the detailed version number is not indicated in the filename, 
- so we need to use a script to read the current version of GLIBCXX in the environment to see if it meets the expectations.

Co-authored-by:  chengy-sysu <939416532@qq.com>
@NorthSecond NorthSecond changed the title Fix: Fixed glibc version detection bug in check-requirements-linux.sh (issue #204186) Fix: GLIBCXX version detection bug in check-requirements-linux.sh (issue #204186) Feb 7, 2024
Copy link
Contributor

@deepak1556 deepak1556 left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, some follow-ups

  1. Can you provide the output of bash -x check-requirements-linux.sh without the changes from this PR.
  2. strings will not available by default on many cases so we would need a fallback

@deepak1556 deepak1556 assigned deepak1556 and unassigned roblourens Feb 7, 2024
Since some Linux distributions do not come with GNU binutils pre-installed, 
the `strings` command does not fit on all platforms, 
so we use `grep` and `sed` instead of `strings`.

Co-authored-by: chengy-sysu <939416532@qq.com>
@NorthSecond
Copy link
Contributor Author

Thanks for the PR, some follow-ups

  1. Can you provide the output of bash -x check-requirements-linux.sh without the changes from this PR.
  2. strings will not available by default on many cases so we would need a fallback
  1. The result of previous bash -x check-requirements-linux.sh is Warning: Missing GLIBCXX >= 3.4.25! from /usr/lib/x86_64-linux-gnu/libstdc++.so.6.
  2. Since some Linux distributions do not come with GNU binutils pre-installed, the strings command does not fit on all platforms, so we use grep and sed instead of strings. However, this change may cause performance degradation.

Co-authored-by: chengy-sysu <939416532@qq.com>
@deepak1556
Copy link
Contributor

Sorry the change still uses strings, can you please provide a minimal devcontainer configuration that would demonstrate the error you are seeing.

Since some Linux distributions do not come with GNU binutils pre-installed, 
the `strings` command does not fit on all platforms, 
so we use `grep` and `sed` instead of `strings`.

Co-authored-by: chengy-sysu <939416532@qq.com>
@NorthSecond
Copy link
Contributor Author

Sorry the change still uses strings, can you please provide a minimal devcontainer configuration that would demonstrate the error you are seeing.

Apologies for the fact that due to the changes I copied from vim, I didn't succeed in pasting them into the online editor when editing, now the current version no longer requires strings.

I tested this using a real Linux SSH connection to the server - in fact, it was a workstation with an Ubuntu 22.04 system. I was not involved in the installation and maintenance of the environment, so you may want to contact the author of issue #204186 if you need a reproducible container environment.

@deepak1556
Copy link
Contributor

Issue #204186 is different from the issue you are describing, there the script resolved to proper files

Warning: Missing GLIBCXX >= 3.4.25! from /usr/lib64/libstdc++.so.6.0.24
Warning: Missing GLIBC >= 2.28! from /usr/lib64/libc-2.26.so

But based on the ldd output in #204186 (comment) it is very likely the author has multiple toolchain installations, so we would need to understand why that is the case and resolve that.

If this PR is trying to address #204186 then let's wait for #204186 (comment). Also the script has got many changes since the issue was reported so I would also like to try the latest insiders on the setup.

@NorthSecond
Copy link
Contributor Author

NorthSecond commented Feb 8, 2024

Issue #204186 is different from the issue you are describing, there the script resolved to proper files

Warning: Missing GLIBCXX >= 3.4.25! from /usr/lib64/libstdc++.so.6.0.24
Warning: Missing GLIBC >= 2.28! from /usr/lib64/libc-2.26.so

But based on the ldd output in #204186 (comment) it is very likely the author has multiple toolchain installations, so we would need to understand why that is the case and resolve that.

If this PR is trying to address #204186 then let's wait for #204186 (comment). Also the script has got many changes since the issue was reported so I would also like to try the latest insiders on the setup.

Okay, so it looks like the issue is different from what I'm experiencing.

Going back to my environment, when I run ls -l | grep libstdc++.so.6 in the /usr/lib/x86 64-linux-gnu directory, I get a somehow "unusual" output:

-rwxr-xr-x  2 root root  12212840 Jan  12 01:15 libstdc++.so.6
-rwxr-xr-x  2 root root  12212840 Jan  12 01:15 libstdc++.so.6.0.30

Run cmp ./libstdc++.so.6.0.30 ./libstdc++.so.6 and there is no output, indicating that the contents of the two files are identical.

So the reason of the problem is that under the current system, instead of soft linking libstdc++.so.6 to libstdc++.so.6.0.30 with ln as expected, the solution in the current environment is to copy it directly, which directly results in the current libstdc++.so.6 failing to pass the environment check.

Skimmed GNU C++ Library Documentation. I didn't find any indication that libstdc++.so.6 is necessarily a linked file, just that there is a libstdc++.so.6 in LD_LIBRARY_PATH.

Therefore, I prefer to use the internal contents of libstdc++.so.6 to determine if GLIBCXX_3.4.25 is actually supported, rather than renaming it with the cp command to easily get older versions of the GLIBCXX to pass inspection.

@NorthSecond
Copy link
Contributor Author

Then, another problem is that the script looks for the libstdc++.so.6 file directly in /usr/lib and /usr/lib64 , but for some environments with multiple toolchain installations, we may manually use tools like patchelf to specify other versions of libstdc++.so.6 for VSCode. other versions of libstdc++.so.6, which may result in the detection script not executing as expected.

@deepak1556
Copy link
Contributor

deepak1556 commented Feb 8, 2024

Thanks for the details, quick clarification have you tested the latest version of the script from main branch ? We actually try the files from the ld cache before using the hardcoded paths which should handle multiple installations

if [ "$OS_ID" != "alpine" ]; then
if [ -f /sbin/ldconfig ]; then
# Look up path
libstdcpp_paths=$(/sbin/ldconfig -p | grep 'libstdc++.so.6')
if [ "$(echo "$libstdcpp_paths" | wc -l)" -gt 1 ]; then
libstdcpp_path=$(echo "$libstdcpp_paths" | grep "$LDCONFIG_ARCH" | awk '{print $NF}')
else
libstdcpp_path=$(echo "$libstdcpp_paths" | awk '{print $NF}')
fi
fi
fi

@deepak1556
Copy link
Contributor

What does ldconfig -p output in your setup ?

@NorthSecond
Copy link
Contributor Author

NorthSecond commented Feb 8, 2024

What does ldconfig -p output in your setup ?

The result of ldconfig -p | grep libstdc++ is

libstdc++.so.6 (libc6,x86-64) => /lib/x86_64-linux-gnu/libstdc++.so.6
libstdc++.so.6 (libc6) => /lib/i386-linux-gnu/libstdc++.so.6

@deepak1556
Copy link
Contributor

Thanks, I am good with this change given the context. Let me test with a couple of different setups before merging.

@deepak1556
Copy link
Contributor

There are conflicts in this PR, can you please rebase and resolve them.

@NorthSecond
Copy link
Contributor Author

Thanks for the details, quick clarification have you tested the latest version of the script from main branch ? We actually try the files from the ld cache before using the hardcoded paths which should handle multiple installations

I used the following section from the latest version for my tests

if [ "$OS_ID" != "alpine" ]; then
if [ -f /sbin/ldconfig ]; then
# Look up path
libstdcpp_paths=$(/sbin/ldconfig -p | grep 'libstdc++.so.6')
if [ "$(echo "$libstdcpp_paths" | wc -l)" -gt 1 ]; then
libstdcpp_path=$(echo "$libstdcpp_paths" | grep "$LDCONFIG_ARCH" | awk '{print $NF}')
else
libstdcpp_path=$(echo "$libstdcpp_paths" | awk '{print $NF}')
fi
fi
fi
if [ -z "$libstdcpp_path" ]; then
if [ -f /usr/lib/libstdc++.so.6 ]; then
# Typical path
libstdcpp_path='/usr/lib/libstdc++.so.6'
elif [ -f /usr/lib64/libstdc++.so.6 ]; then
# Typical path
libstdcpp_path='/usr/lib64/libstdc++.so.6'
else
echo "Warning: Can't find libstdc++.so or ldconfig, can't verify libstdc++ version"
fi
fi
while [ -n "$libstdcpp_path" ]; do
# Extracts the version number from the path, e.g. libstdc++.so.6.0.22 -> 6.0.22
# which is then compared based on the fact that release versioning and symbol versioning
# are aligned for libstdc++. Refs https://gcc.gnu.org/onlinedocs/libstdc++/manual/abi.html
# (i-e) GLIBCXX_3.4.<release> is provided by libstdc++.so.6.y.<release>
libstdcpp_path_line=$(echo "$libstdcpp_path" | head -n1)
libstdcpp_real_path=$(readlink -f "$libstdcpp_path_line")
libstdcpp_version=$(echo "$libstdcpp_real_path" | awk -F'\\.so\\.' '{print $NF}')
if [ "$(printf '%s\n' "6.0.25" "$libstdcpp_version" | sort -V | head -n1)" = "6.0.25" ]; then
found_required_glibcxx=1
break
fi
libstdcpp_path=$(echo "$libstdcpp_path" | tail -n +2) # remove first line
done
if [ "$found_required_glibcxx" = "0" ]; then
echo "Warning: Missing GLIBCXX >= 3.4.25! from $libstdcpp_real_path"
fi

For analysis purposes, I have included two echo statements as follows:

image

The result obtained by running it is:

libstdcpp_real_path: /usr/lib/x86_64-linux-gnu/libstdc++.so.6
version: 6
libstdcpp_real_path: /usr/lib/i386-linux-gnu/libstdc++.so.6.0.28
version: 6.0.28

The new version has a new problem: it incorrectly reads and adopts the i386 toolchain library.

NorthSecond and others added 2 commits February 8, 2024 18:18
Co-authored-by: chengy-sysu <939416532@qq.com>
@NorthSecond
Copy link
Contributor Author

There are conflicts in this PR, can you please rebase and resolve them.

Solved it.

Copy link
Contributor

@deepak1556 deepak1556 left a comment

Choose a reason for hiding this comment

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

LGTM, I add a small change to skip the new check on alpine since it will not work there. Please check if the script continues to work for your setup.

@VSCodeTriageBot VSCodeTriageBot added this to the February 2024 milestone Feb 8, 2024
@NorthSecond
Copy link
Contributor Author

LGTM, I add a small change to skip the new check on alpine since it will not work there. Please check if the script continues to work for your setup.

Yes It work's well.

@NorthSecond
Copy link
Contributor Author

@NorthSecond please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.

@microsoft-github-policy-service agree [company="{your company}"]

Options:

  • (default - no company specified) I have sole ownership of intellectual property rights to my Submissions and I am not making Submissions in the course of work for my employer.
@microsoft-github-policy-service agree
  • (when company given) I am making Submissions in the course of work for my employer (or my employer has intellectual property rights in my Submissions by contract or applicable law). I have permission from my employer to make Submissions and enter into this Agreement on behalf of my employer. By signing below, the defined term “You” includes me and my employer.
@microsoft-github-policy-service agree company="Microsoft"

Contributor License Agreement

@microsoft-github-policy-service agree

@NorthSecond
Copy link
Contributor Author

Thanks for the review and the conversation, I've updated the current PR to based on the latest main branch.

Look forward to the next collaboration!

@deepak1556 deepak1556 merged commit a12d9f4 into microsoft:main Feb 8, 2024
6 checks passed
@NorthSecond NorthSecond deleted the patch-1 branch February 8, 2024 11:44
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.

None yet

5 participants