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.

Sign up for GitHub

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

Jump to bottom

runtime: minor bugs fixed in coreclr and libraries #97155

Merged
merged 9 commits into from
Jan 19, 2024
Merged

runtime: minor bugs fixed in coreclr and libraries #97155

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
5fcb82d
Fixed error: <Pointer, returned from function 'gmtime_r' at filetime.…
kaybelev Jan 18, 2024
09fd2f5
Fixed error: <Return value of a function 'PAL_wcsrchr' is dereference…
kaybelev Jan 18, 2024
3369cbe
Fixed error: <Value sectionRecord, which is result of method invocati…
kaybelev Jan 18, 2024
d32b505
Fixed error: <Maybe ecmaCandidateMethod, that created from method wit…
kaybelev Jan 18, 2024
82d4ff2
Update filetime.cpp
kaybelev Jan 19, 2024
202117d
Update process.cpp
kaybelev Jan 19, 2024
41ed108
Update MgmtConfigurationRecord.cs
kaybelev Jan 19, 2024
11e19ea
Update src/libraries/System.Configuration.ConfigurationManager/src/Sy…
kaybelev Jan 19, 2024
a7282df
Update src/libraries/System.Configuration.ConfigurationManager/src/Sy…
kaybelev Jan 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/coreclr/pal/src/file/filetime.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,7 @@ BOOL PALAPI FileTimeToSystemTime( CONST FILETIME * lpFileTime,
#else /* HAVE_GMTIME_R */
UnixSystemTime = gmtime( &UnixFileTime );
#endif /* HAVE_GMTIME_R */
if (!UnixSystemTime) return FALSE;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if (!UnixSystemTime) return FALSE;
if (!UnixSystemTime)
{
ERROR( "gmtime failed.\n" );
SetLastError(ERROR_INVALID_PARAMETER);
return FALSE;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Jan, thanks for the correction, we'll take it into account for the future.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Commit updated.


/* Convert unix system time to Windows system time. */
lpSystemTime->wDay = (WORD)UnixSystemTime->tm_mday;
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/pal/src/thread/process.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2821,6 +2821,7 @@ CorUnix::InitializeProcessCommandLine(
if (lpwstrFullPath)
{
LPWSTR lpwstr = PAL_wcsrchr(lpwstrFullPath, '/');
if (!lpwstr) {palError = ERROR_INTERNAL_ERROR; goto exit;}
kaybelev marked this conversation as resolved.
Show resolved Hide resolved
lpwstr[0] = '\0';
size_t n = PAL_wcslen(lpwstrFullPath) + 1;

Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/tools/aot/ILCompiler.ReadyToRun/IBC/IBCProfileParser.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -414,7 +414,7 @@ private uint LookupIbcMethodToken(MetadataType methodMetadataType, uint ibcToken
if (method.Name == methodName)
{
EcmaMethod ecmaCandidateMethod = method as EcmaMethod;
if (method == null)
if (ecmaCandidateMethod == null)
continue;

MetadataReader metadataReader = ecmaCandidateMethod.MetadataReader;
Expand Down
2 changes: 1 addition & 1 deletion ...em.Configuration.ConfigurationManager/src/System/Configuration/MgmtConfigurationRecord.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,7 @@ internal ConfigurationSection FindImmediateParentSection(ConfigurationSection se

string configKey = section.SectionInformation.SectionName;
SectionRecord sectionRecord = GetSectionRecord(configKey, false);
if (sectionRecord.HasLocationInputs)
if ((sectionRecord != null) && sectionRecord.HasLocationInputs)
Copy link
Member

Choose a reason for hiding this comment

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

This doesn't actually fix a null reference exception. If sectionRecord could actually be null, this will cause the else block to be executed, which also immediately dereferences sectionRecord.

Unless there's a known repro for this causing a problem, let's revert this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Stefan, to correct the error, I propose below in line 346 to change the condition to <if ((sectionRecord != null) && sectionRecord.HasIndirectLocationInputs)>. Stefan, judging by the results of the static analysis, the value returned by the function GetSectionRecord(...) in the library System.Configuration.ConfigurationManager code base should be checked for NULL (you can search (sectionRecord==null or sectionRecord!=null) by searching in the files MgmtConfigurationRecord.cs and BaseConfigurationRecord.cs). I suggest you fix it. It is not possible to check for an error in this case using a test, because it was detected using a static analysis tool.

Copy link
Contributor Author

@kaybelev kaybelev Jan 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Commit updated.

Copy link
Member

Choose a reason for hiding this comment

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

This method is searching for immediate parent section. I assume that the immediate parent section is expected to always exist and that's why there is no null check. Maybe we can add Debug.Assert instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, let's try, just explain what needs to be done for this.

kaybelev marked this conversation as resolved.
Show resolved Hide resolved
{
SectionInput input = sectionRecord.LastLocationInput;
Debug.Assert(input.HasResult, "input.HasResult");
Expand Down
Loading