Skip to content

Fix sal warnings #2279

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

Merged
merged 1 commit into from
Dec 23, 2016
Merged

Fix sal warnings #2279

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 3 additions & 0 deletions lib/Backend/CodeGenNumberAllocator.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -324,6 +324,9 @@ Js::JavascriptNumber* XProcNumberPageSegmentImpl::AllocateNumber(Func* func, dou
Recycler::FillPadNoCheck(pLocalNumber, sizeof(Js::JavascriptNumber), sizeCat, false);
pLocalNumber = new (pLocalNumber) Js::JavascriptNumber(localNumber);
}
#else
Assert(sizeCat == sizeof(Js::JavascriptNumber));
__analysis_assume(sizeCat == sizeof(Js::JavascriptNumber));
Copy link
Collaborator

@ThomsonTan ThomsonTan Dec 22, 2016

Choose a reason for hiding this comment

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

__analysis_assume(sizeCat == sizeof(Js::JavascriptNumber)); [](start = 12, length = 59)

Why only need this if RECYCLER_MEMORY_VERIFY is not defined? I don't see it is set when RECYCLER_MEMORY_VERIFY is defined.

Copy link
Contributor

@leirocks leirocks Dec 22, 2016

Choose a reason for hiding this comment

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

RECYCLER_MEMORY_VERIFY means it's checked build, which looks we don't run pre-fast with

Copy link
Contributor Author

@meg-gupta meg-gupta Dec 22, 2016

Choose a reason for hiding this comment

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

There is no warning when RECYCLER_MEMORY_VERIFY is defined, pLocalNumber is allocated as :
pLocalNumber = (Js::JavascriptNumber*)alloca(sizeCat);
So at WriteProcessMemory(hProcess, (void*)number, pLocalNumber, sizeCat, NULL) there will be no warning from the analyzer, saying pLocalNumber should be >= sizeCat.

Also looks like when RECYCLER_MEMORY_VERIFY is defined sizeCat can be larger than sizeof(Js::JavascriptNumber).

#endif
// change vtable to the remote one
*(void**)pLocalNumber = (void*)func->GetScriptContextInfo()->GetVTableAddress(VTableValue::VtableJavascriptNumber);
Expand Down
5 changes: 3 additions & 2 deletions lib/Backend/Encoder.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -295,8 +295,9 @@ Encoder::Encode()
{
if (m_func->IsOOPJIT())
{
Js::ThrowMapEntry * throwMap = NativeCodeDataNewArrayNoFixup(m_func->GetNativeCodeDataAllocator(), Js::ThrowMapEntry, m_pragmaInstrToRecordMap->Count());
for (int32 i = 0; i < m_pragmaInstrToRecordMap->Count(); i++)
int allocSize = m_pragmaInstrToRecordMap->Count();
Copy link
Collaborator

@ThomsonTan ThomsonTan Dec 22, 2016

Choose a reason for hiding this comment

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

int [](start = 12, length = 3)

Use int32 to declare allocSize to be consistent?

Copy link
Contributor Author

@meg-gupta meg-gupta Dec 22, 2016

Choose a reason for hiding this comment

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

Done

Js::ThrowMapEntry * throwMap = NativeCodeDataNewArrayNoFixup(m_func->GetNativeCodeDataAllocator(), Js::ThrowMapEntry, allocSize);
Copy link
Collaborator

@ThomsonTan ThomsonTan Dec 22, 2016

Choose a reason for hiding this comment

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

i < m_pragmaInstrToRecordMap->Count() [](start = 30, length = 37)

What's the problem in i < m_progmaInstrToRecordMap, type mismatch?

Copy link
Contributor Author

@meg-gupta meg-gupta Dec 22, 2016

Choose a reason for hiding this comment

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

The calls to m_pragmaInstrToRecordMap->Count() for AllocatorArray and loop counter is not recognized as equivalent by the analyzer.

for (int i = 0; i < allocSize; i++)
{
IR::PragmaInstr *inst = m_pragmaInstrToRecordMap->Item(i);
throwMap[i].nativeBufferOffset = inst->m_offsetInBuffer;
Expand Down