Skip to content

Commit 1e166fb

Browse files
authored
Increase Native build warning level to W3 and fix build warnings (#3370)
* Reverted to Warning level 3 for unmanaged builds * fixed three W3 security-critical warnings that were deemed security critical, but false positives * Added safety checks for array initializers in 32-bit platform for LDA Native. Prior code did not check allocation arrays correctly on 32-bit platforms. * Fixed signed/unsigned warnings * fixed warnings on Alias_multinomial_rng_int.gpp * Removed error handling as the exception is not x-platform. * update all space and formatting to <spaces> * update SSE.cpp comments to reflect correct limits of a variable -- checked for one more overflow condition -- ensure all exceptions are thrown * Added one more allocation check - standardized to numeric_limits of int32_t * fixed a clang-only warning * fix size comments
1 parent ea5c095 commit 1e166fb

File tree

10 files changed

+97
-62
lines changed

10 files changed

+97
-62
lines changed

src/Native/CMakeLists.txt

-1
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@ if(WIN32)
3232
add_compile_options(/FC) # use full pathnames in diagnostics
3333
add_compile_options(/DEBUG)
3434
add_compile_options(/GS)
35-
add_compile_options(/W1)
3635
add_compile_options(/Zc:inline)
3736
add_compile_options(/fp:precise)
3837
add_compile_options(/EHsc)

src/Native/CpuMathNative/Sse.cpp

+12-10
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,7 @@ EXPORT_API(void) MatMulTran(_In_ const float * pmat, _In_ const float * psrc, _I
158158
}
159159

160160
pm += 3 * crow;
161-
161+
162162
for (; ps < psLim; ps += 4)
163163
{
164164
__m128 x01 = _mm_load_ps(ps);
@@ -219,9 +219,9 @@ EXPORT_API(void) Scale(float a, _Inout_ float * pd, int c)
219219
{
220220
switch (c)
221221
{
222-
case 3: pd[2] *= a;
223-
case 2: pd[1] *= a;
224-
case 1: pd[0] *= a;
222+
case 3: pd[2] *= a;
223+
case 2: pd[1] *= a;
224+
case 1: pd[0] *= a;
225225
}
226226
return;
227227
}
@@ -266,7 +266,8 @@ EXPORT_API(void) Scale(float a, _Inout_ float * pd, int c)
266266
_mm_storeu_ps(pd, result);
267267

268268
pd += misalignment;
269-
c -= misalignment;
269+
// safe to downcast as misalignment <= 128.
270+
c -= static_cast<int>(misalignment);
270271
}
271272

272273
if (c > 3)
@@ -489,9 +490,9 @@ EXPORT_API(float) Sum(const float* pValues, int length)
489490

490491
switch (length)
491492
{
492-
case 3: result += pValues[2];
493-
case 2: result += pValues[1];
494-
case 1: result += pValues[0];
493+
case 3: result += pValues[2];
494+
case 2: result += pValues[1];
495+
case 1: result += pValues[0];
495496
}
496497

497498
return result;
@@ -532,7 +533,8 @@ EXPORT_API(float) Sum(const float* pValues, int length)
532533
result = _mm_add_ps(result, temp);
533534

534535
pValues += misalignment;
535-
length -= misalignment;
536+
// safe to downcast as misalignment < 16.
537+
length -= static_cast<int>(misalignment);
536538
}
537539

538540
if (length > 3)
@@ -882,4 +884,4 @@ EXPORT_API(void) SdcaL1UpdateSU(float primalUpdate, _In_ const float * ps, _In_
882884
float d1 = pd1[i];
883885
pd2[i] = std::abs(d1) > threshold ? (d1 > 0 ? d1 - threshold : d1 + threshold) : 0;
884886
}
885-
}
887+
}

src/Native/LdaNative/alias_multinomial_rng_int.cpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -23,13 +23,13 @@ namespace wood
2323
delete[]internal_memory_;
2424
}
2525
}
26-
26+
2727
int32_t AliasMultinomialRNGInt::Next(xorshift_rng& rng, std::vector<alias_k_v>& alias_kv)
2828
{
2929
// NOTE: stl uniform_real_distribution generates the highest quality random numbers
3030
// yet, the other two are much faster
3131
auto sample = rng.rand();
32-
32+
3333
// NOTE: use std::floor is too slow
3434
// here we guarantee sample * n_ is nonnegative, this makes cast work
3535
int idx = sample / a_int_;

src/Native/LdaNative/alias_multinomial_rng_int.hpp

+11-6
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
#include <iostream>
1515
#include <assert.h>
1616
/*
17-
Algorithm described in
17+
Algorithm described in
1818
https://www.jstatsoft.org/v11/i03/paper
1919
George Marsaglia
2020
Fast generation of discrete random variables
@@ -108,7 +108,12 @@ namespace wood
108108
int32_t H_head = 0;
109109
int32_t H_tail = 0;
110110

111-
for (auto i = 0; i < proportion_int_.size(); ++i)
111+
112+
// note that i must fit into int32_t of L_[L_tail].first
113+
if (static_cast<uint32_t>(std::numeric_limits<int32_t>::max()) < proportion_int_.size() )
114+
throw std::bad_alloc();
115+
int32_t size = static_cast<int32_t>(proportion_int_.size());
116+
for (int32_t i = 0; i < size; ++i)
112117
{
113118
auto val = proportion_int_[i];
114119
if (val < a_int_)
@@ -154,7 +159,7 @@ namespace wood
154159
auto first = L_[L_head].first;
155160
auto second = L_[L_head].second;
156161
alias_kv[first].k_ = first;
157-
alias_kv[first].v_ = first * a_int_ + second;
162+
alias_kv[first].v_ = first * a_int_ + second;
158163
++L_head;
159164
}
160165
while (H_head != H_tail)
@@ -227,7 +232,7 @@ namespace wood
227232
*p = i; p++;
228233
*p = (i + 1) * a_int_;
229234
}
230-
235+
231236
int32_t L_head = 0;
232237
int32_t L_tail = 0;
233238

@@ -295,8 +300,8 @@ namespace wood
295300
*p = first; p++;
296301
*p = first * a_int_ + second;
297302
++H_head;
298-
}
299-
memcpy(memory, internal_memory_, sizeof(int32_t)* 2 * n_);
303+
}
304+
memcpy(memory, internal_memory_, sizeof(int32_t) * 2 * n_);
300305
}
301306

302307
inline void SetProportionMass(std::vector<float> &proportion,

src/Native/LdaNative/data_block.cpp

+14-4
Original file line numberDiff line numberDiff line change
@@ -3,17 +3,21 @@
33
// See the LICENSE file in the project root for more information.
44

55
#include <iostream>
6+
#include <stdexcept>
7+
#include <limits>
68
#include "data_block.h"
79
#include "lda_document.h"
810

911
namespace lda
1012
{
11-
LDADataBlock::LDADataBlock(int32_t num_threads) :
13+
using namespace std;
14+
15+
LDADataBlock::LDADataBlock(int32_t num_threads) :
1216
num_threads_(num_threads), has_read_(false), index_document_(0), documents_buffer_(nullptr), offset_buffer_(nullptr)
1317
{
1418
}
1519

16-
LDADataBlock::~LDADataBlock()
20+
LDADataBlock::~LDADataBlock()
1721
{
1822
if (has_read_)
1923
{
@@ -46,7 +50,13 @@ namespace lda
4650
void LDADataBlock::Allocate(const int32_t num_document, const int64_t corpus_size)
4751
{
4852
num_documents_ = num_document;
49-
corpus_size_ = corpus_size;
53+
54+
if (corpus_size < 0 || static_cast<uint64_t>(corpus_size) > numeric_limits<size_t>::max())
55+
throw bad_alloc();
56+
corpus_size_ = static_cast<size_t>(corpus_size);
57+
58+
if (num_documents_ < 0 || static_cast<uint64_t>(num_documents_) >(numeric_limits<size_t>::max() - 1))
59+
throw bad_alloc();
5060

5161
offset_buffer_ = new int64_t[num_documents_ + 1]; // +1: one for the end of last document,
5262
documents_buffer_ = new int32_t[corpus_size_];
@@ -86,7 +96,7 @@ namespace lda
8696
int LDADataBlock::AddDense(int32_t* term_freq, int32_t term_num)
8797
{
8898
int64_t data_length = 1;
89-
99+
90100
int64_t idx = offset_buffer_[index_document_] + 1;
91101
for (int i = 0; i < term_num; ++i)
92102
{

src/Native/LdaNative/data_block.h

+5-5
Original file line numberDiff line numberDiff line change
@@ -17,18 +17,18 @@ namespace lda
1717
public:
1818
explicit LDADataBlock(int32_t num_threads);
1919
~LDADataBlock();
20-
20+
2121
void Clear();
2222
//in data feedin scenario
23-
void Allocate(const int32_t num_document, const int64_t corpus_size);
23+
void Allocate(const int32_t num_document, const int64_t corpus_size);
2424
//port the data from external process, for example, c#
2525
int AddDense(int32_t* term_freq, int32_t term_num);
2626
int Add(int32_t* term_id, int32_t* term_freq, int32_t term_num);
2727
std::shared_ptr<LDADocument> GetOneDoc(int32_t index) const;
2828

2929
inline int32_t num_documents() const;
3030
// Return the first document for thread thread_id
31-
inline int32_t Begin(int32_t thread_id) const;
31+
inline int32_t Begin(int32_t thread_id) const;
3232
// Return the next to last document for thread thread_i
3333
inline int32_t End(int32_t thread_id) const;
3434

@@ -43,8 +43,8 @@ namespace lda
4343
int32_t index_document_;
4444
int64_t used_size_;
4545

46-
int32_t num_documents_;
47-
int64_t corpus_size_;
46+
int32_t num_documents_;
47+
size_t corpus_size_;
4848

4949
int64_t* offset_buffer_; // offset_buffer_ size = num_document_ + 1
5050
int32_t* documents_buffer_; // documents_buffer_ size = corpus_size_;

src/Native/LdaNative/lda_engine.cpp

+3-3
Original file line numberDiff line numberDiff line change
@@ -377,7 +377,7 @@ namespace lda {
377377
std::vector<std::pair<int, double>> llcontainer;
378378
// Set core affinity which helps performance improvement
379379
#ifdef _MSC_VER
380-
long long maskLL = 0;
380+
DWORD maskLL = 0;
381381
maskLL |= (1LL << (thread_id));
382382
DWORD_PTR mask = maskLL;
383383
SetThreadAffinityMask(GetCurrentThread(), mask);
@@ -542,7 +542,7 @@ namespace lda {
542542
if (thread_id == 0)
543543
{
544544
//output the ll once
545-
for (int i = 0; i < llcontainer.size(); i++)
545+
for (size_t i = 0; i < llcontainer.size(); i++)
546546
{
547547
printf("loglikelihood @iter%04d = %f\n", llcontainer[i].first, llcontainer[i].second);
548548
}
@@ -560,7 +560,7 @@ namespace lda {
560560

561561
// Set core affinity which helps performance improvement
562562
#ifdef _MSC_VER
563-
long long maskLL = 0;
563+
DWORD maskLL = 0;
564564
maskLL |= (1LL << (thread_id));
565565
DWORD_PTR mask = maskLL;
566566
SetThreadAffinityMask(GetCurrentThread(), mask);

0 commit comments

Comments
 (0)