Skip to content

Commit

Permalink
Fix failures on Windows when more warnings are enabled. (#49194)
Browse files Browse the repository at this point in the history
* Fix warning for System.IO.Compression.Native when more analysis is enabled.

* Fix warnings in corehost when more analysis is enabled.
  • Loading branch information
AaronRobinsonMSFT authored Mar 19, 2021
1 parent c999ed4 commit 3df7a6e
Show file tree
Hide file tree
Showing 19 changed files with 77 additions and 50 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1540,7 +1540,7 @@ z_streamp source;
struct inflate_state FAR *state;
struct inflate_state FAR *copy;
unsigned char FAR *window;
unsigned wsize;
unsigned wsize = 0;

/* check input */
if (inflateStateCheck(source) || dest == Z_NULL)
Expand Down
10 changes: 5 additions & 5 deletions src/native/corehost/bundle/extractor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ pal::string_t& extractor_t::extraction_dir()
if (m_extraction_dir.empty())
{
// Compute the final extraction location as:
// m_extraction_dir = $DOTNET_BUNDLE_EXTRACT_BASE_DIR/<app>/<id>/...
//
// m_extraction_dir = $DOTNET_BUNDLE_EXTRACT_BASE_DIR/<app>/<id>/...
//
// If DOTNET_BUNDLE_EXTRACT_BASE_DIR is not set in the environment,
// a default is choosen within the temporary directory.

Expand Down Expand Up @@ -103,9 +103,9 @@ void extractor_t::extract(const file_entry_t &entry, reader_t &reader)
{
FILE* file = create_extraction_file(entry.relative_path());
reader.set_offset(entry.offset());
size_t size = entry.size();

if (fwrite(reader, 1, size, file) != size)
int64_t size = entry.size();
size_t cast_size = to_size_t_dbgchecked(size);
if (fwrite(reader, 1, cast_size, file) != cast_size)
{
trace::error(_X("Failure extracting contents of the application bundle."));
trace::error(_X("I/O failure when writing extracted files."));
Expand Down
3 changes: 2 additions & 1 deletion src/native/corehost/bundle/reader.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

#include <cstdint>
#include "pal.h"
#include "utils.h"

namespace bundle
{
Expand Down Expand Up @@ -40,7 +41,7 @@ namespace bundle
void read(void* dest, int64_t len)
{
bounds_check(len);
memcpy(dest, m_ptr, len);
memcpy(dest, m_ptr, to_size_t_dbgchecked(len));
m_ptr += len;
}

Expand Down
6 changes: 3 additions & 3 deletions src/native/corehost/deps_format.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ void deps_json_t::reconcile_libraries_with_targets(
const pal::string_t& deps_path,
const json_parser_t::value_t& json,
const std::function<bool(const pal::string_t&)>& library_exists_fn,
const std::function<const vec_asset_t&(const pal::string_t&, int, bool*)>& get_assets_fn)
const std::function<const vec_asset_t&(const pal::string_t&, size_t, bool*)>& get_assets_fn)
{
pal::string_t deps_file = get_filename(deps_path);

Expand Down Expand Up @@ -320,7 +320,7 @@ bool deps_json_t::load_framework_dependent(const pal::string_t& deps_path, const
};

const vec_asset_t empty;
auto get_relpaths = [&](const pal::string_t& package, int asset_type_index, bool* rid_specific) -> const vec_asset_t& {
auto get_relpaths = [&](const pal::string_t& package, size_t asset_type_index, bool* rid_specific) -> const vec_asset_t& {

*rid_specific = false;

Expand Down Expand Up @@ -361,7 +361,7 @@ bool deps_json_t::load_self_contained(const pal::string_t& deps_path, const json
return m_assets.libs.count(package);
};

auto get_relpaths = [&](const pal::string_t& package, int type_index, bool* rid_specific) -> const vec_asset_t& {
auto get_relpaths = [&](const pal::string_t& package, size_t type_index, bool* rid_specific) -> const vec_asset_t& {
*rid_specific = false;
return m_assets.libs[package][type_index];
};
Expand Down
2 changes: 1 addition & 1 deletion src/native/corehost/deps_format.h
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ class deps_json_t
const pal::string_t& deps_path,
const json_parser_t::value_t& json,
const std::function<bool(const pal::string_t&)>& library_exists_fn,
const std::function<const vec_asset_t&(const pal::string_t&, int, bool*)>& get_assets_fn);
const std::function<const vec_asset_t&(const pal::string_t&, size_t, bool*)>& get_assets_fn);

pal::string_t get_current_rid(const rid_fallback_graph_t& rid_fallback_graph);
bool perform_rid_fallback(rid_specific_assets_t* portable_assets, const rid_fallback_graph_t& rid_fallback_graph);
Expand Down
9 changes: 7 additions & 2 deletions src/native/corehost/fxr/corehost_init.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ corehost_init_t::corehost_init_t(
{
make_cstr_arr(m_probe_paths, &m_probe_paths_cstr);

int fx_count = fx_definitions.size();
size_t fx_count = fx_definitions.size();
m_fx_names.reserve(fx_count);
m_fx_dirs.reserve(fx_count);
m_fx_requested_versions.reserve(fx_count);
Expand Down Expand Up @@ -132,7 +132,12 @@ const host_interface_t& corehost_init_t::get_host_init_data()
hi.host_info_dotnet_root = m_host_info_dotnet_root.c_str();
hi.host_info_app_path = m_host_info_app_path.c_str();

hi.single_file_bundle_header_offset = bundle::info_t::is_single_file_bundle() ? bundle::info_t::the_app->header_offset() : 0;
hi.single_file_bundle_header_offset = 0;
if (bundle::info_t::is_single_file_bundle())
{
int64_t offset = bundle::info_t::the_app->header_offset();
hi.single_file_bundle_header_offset = to_size_t_dbgchecked(offset);
}

return hi;
}
Expand Down
10 changes: 5 additions & 5 deletions src/native/corehost/fxr/fx_muxer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -849,7 +849,7 @@ int fx_muxer_t::run_app(host_context_t *context)
if (!context->is_app)
return StatusCode::InvalidArgFailure;

int argc = context->argv.size();
size_t argc = context->argv.size();
std::vector<const pal::char_t*> argv;
argv.reserve(argc);
for (const auto& str : context->argv)
Expand All @@ -863,7 +863,7 @@ int fx_muxer_t::run_app(host_context_t *context)
if (rc != StatusCode::Success)
return rc;

return contract.run_app(argc, argv.data());
return contract.run_app((int32_t)argc, argv.data());
}
}

Expand Down Expand Up @@ -992,7 +992,7 @@ int fx_muxer_t::handle_exec_host_command(
vec_argv.push_back(argv[0]);
vec_argv.insert(vec_argv.end(), argv + argoff, argv + argc);
new_argv = vec_argv.data();
new_argc = vec_argv.size();
new_argc = (int32_t)vec_argv.size();
}

trace::info(_X("Using dotnet root path [%s]"), host_info.dotnet_root.c_str());
Expand Down Expand Up @@ -1083,7 +1083,7 @@ int fx_muxer_t::handle_cli(
int new_argoff;
pal::string_t sdk_app_candidate;
opt_map_t opts;
int result = command_line::parse_args_for_sdk_command(host_info, new_argv.size(), new_argv.data(), &new_argoff, sdk_app_candidate, opts);
int result = command_line::parse_args_for_sdk_command(host_info, (int32_t)new_argv.size(), new_argv.data(), &new_argoff, sdk_app_candidate, opts);
if (!result)
{
// Transform dotnet [exec] [--additionalprobingpath path] [--depsfile file] [dll] [args] -> dotnet [dll] [args]
Expand All @@ -1092,7 +1092,7 @@ int fx_muxer_t::handle_cli(
host_info,
sdk_app_candidate,
opts,
new_argv.size(),
(int32_t)new_argv.size(),
new_argv.data(),
new_argoff,
host_mode_t::muxer,
Expand Down
6 changes: 3 additions & 3 deletions src/native/corehost/fxr/hostfxr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ SHARED_API int32_t HOSTFXR_CALLTYPE hostfxr_resolve_sdk(
return 0;
}

unsigned long non_negative_buffer_size = static_cast<unsigned long>(buffer_size);
size_t non_negative_buffer_size = static_cast<size_t>(buffer_size);
if (sdk_path.size() < non_negative_buffer_size)
{
size_t length = sdk_path.copy(buffer, non_negative_buffer_size - 1);
Expand All @@ -158,7 +158,7 @@ SHARED_API int32_t HOSTFXR_CALLTYPE hostfxr_resolve_sdk(
trace::info(_X("hostfxr_resolve_sdk received a buffer that is too small to hold the located SDK path."));
}

return sdk_path.size() + 1;
return static_cast<int32_t>(sdk_path.size() + 1);
}

enum hostfxr_resolve_sdk2_flags_t : int32_t
Expand Down Expand Up @@ -325,7 +325,7 @@ SHARED_API int32_t HOSTFXR_CALLTYPE hostfxr_get_available_sdks(
sdk_dirs.push_back(sdk_info.full_path.c_str());
}

result(sdk_dirs.size(), &sdk_dirs[0]);
result(static_cast<int32_t>(sdk_dirs.size()), &sdk_dirs[0]);
}

return StatusCode::Success;
Expand Down
15 changes: 12 additions & 3 deletions src/native/corehost/hostmisc/pal.h
Original file line number Diff line number Diff line change
Expand Up @@ -149,19 +149,28 @@ namespace pal
inline int cstrcasecmp(const char* str1, const char* str2) { return ::_stricmp(str1, str2); }
inline int strcmp(const char_t* str1, const char_t* str2) { return ::wcscmp(str1, str2); }
inline int strcasecmp(const char_t* str1, const char_t* str2) { return ::_wcsicmp(str1, str2); }
inline int strncmp(const char_t* str1, const char_t* str2, int len) { return ::wcsncmp(str1, str2, len); }
inline int strncasecmp(const char_t* str1, const char_t* str2, int len) { return ::_wcsnicmp(str1, str2, len); }
inline int strncmp(const char_t* str1, const char_t* str2, size_t len) { return ::wcsncmp(str1, str2, len); }
inline int strncasecmp(const char_t* str1, const char_t* str2, size_t len) { return ::_wcsnicmp(str1, str2, len); }
inline int pathcmp(const pal::string_t &path1, const pal::string_t &path2) { return strcasecmp(path1.c_str(), path2.c_str()); }
inline string_t to_string(int value) { return std::to_wstring(value); }

inline size_t strlen(const char_t* str) { return ::wcslen(str); }

#pragma warning(suppress : 4996) // error C4996: '_wfopen': This function or variable may be unsafe.
inline FILE * file_open(const string_t& path, const char_t* mode) { return ::_wfopen(path.c_str(), mode); }

inline void file_vprintf(FILE* f, const char_t* format, va_list vl) { ::vfwprintf(f, format, vl); ::fputwc(_X('\n'), f); }
inline void err_fputs(const char_t* message) { ::fputws(message, stderr); ::fputwc(_X('\n'), stderr); }
inline void out_vprintf(const char_t* format, va_list vl) { ::vfwprintf(stdout, format, vl); ::fputwc(_X('\n'), stdout); }

// This API is being used correctly and querying for needed size first.
#pragma warning(suppress : 4996) // error C4996: '_vsnwprintf': This function or variable may be unsafe.
inline int str_vprintf(char_t* buffer, size_t count, const char_t* format, va_list vl) { return ::_vsnwprintf(buffer, count, format, vl); }
inline const char_t* strerror(int errnum) { return ::_wcserror(errnum); }

// Suppressing warning since the 'safe' version requires an input buffer that is unnecessary for
// uses of this function.
#pragma warning(suppress : 4996) // error C4996: '_wcserror': This function or variable may be unsafe.
inline const char_t* strerror(int errnum){ return ::_wcserror(errnum); }

bool pal_utf8string(const string_t& str, std::vector<char>* out);
bool pal_clrstring(const string_t& str, std::vector<char>* out);
Expand Down
25 changes: 14 additions & 11 deletions src/native/corehost/hostmisc/pal.windows.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,13 @@ pal::string_t pal::to_lower(const pal::string_t& in)

pal::string_t pal::get_timestamp()
{
std::time_t t = std::time(0);
std::time_t t = std::time(nullptr);
const std::size_t elems = 100;
char_t buf[elems];
std::wcsftime(buf, elems, _X("%c GMT"), std::gmtime(&t));

tm tm_l{};
::gmtime_s(&tm_l, &t);
std::wcsftime(buf, elems, _X("%c GMT"), &tm_l);

return pal::string_t(buf);
}
Expand Down Expand Up @@ -141,7 +144,7 @@ bool pal::getcwd(pal::string_t* recv)
{
std::vector<pal::char_t> str;
str.resize(result);
result = GetCurrentDirectoryW(str.size(), str.data());
result = GetCurrentDirectoryW(static_cast<uint32_t>(str.size()), str.data());
assert(result <= str.size());
if (result != 0)
{
Expand Down Expand Up @@ -443,8 +446,8 @@ pal::string_t pal::get_current_os_rid_platform()
if ((*pRtlGetVersion)(&osinfo) == 0)
{
// Win7 RID is the minimum supported version.
int majorVer = 6;
int minorVer = 1;
uint32_t majorVer = 6;
uint32_t minorVer = 1;

if (osinfo.dwMajorVersion > majorVer)
{
Expand Down Expand Up @@ -608,18 +611,18 @@ bool pal::get_default_bundle_extraction_base_dir(pal::string_t& extraction_dir)
return realpath(&extraction_dir);
}

static bool wchar_convert_helper(DWORD code_page, const char* cstr, int len, pal::string_t* out)
static bool wchar_convert_helper(DWORD code_page, const char* cstr, size_t len, pal::string_t* out)
{
out->clear();

// No need of explicit null termination, so pass in the actual length.
size_t size = ::MultiByteToWideChar(code_page, 0, cstr, len, nullptr, 0);
size_t size = ::MultiByteToWideChar(code_page, 0, cstr, static_cast<uint32_t>(len), nullptr, 0);
if (size == 0)
{
return false;
}
out->resize(size, '\0');
return ::MultiByteToWideChar(code_page, 0, cstr, len, &(*out)[0], out->size()) != 0;
return ::MultiByteToWideChar(code_page, 0, cstr, static_cast<uint32_t>(len), &(*out)[0], static_cast<uint32_t>(out->size())) != 0;
}

bool pal::pal_utf8string(const pal::string_t& str, std::vector<char>* out)
Expand All @@ -633,7 +636,7 @@ bool pal::pal_utf8string(const pal::string_t& str, std::vector<char>* out)
return false;
}
out->resize(size, '\0');
return ::WideCharToMultiByte(CP_UTF8, 0, str.c_str(), -1, out->data(), out->size(), nullptr, nullptr) != 0;
return ::WideCharToMultiByte(CP_UTF8, 0, str.c_str(), -1, out->data(), static_cast<uint32_t>(out->size()), nullptr, nullptr) != 0;
}

bool pal::pal_clrstring(const pal::string_t& str, std::vector<char>* out)
Expand All @@ -660,7 +663,7 @@ bool pal::realpath(string_t* path, bool skip_error_logging)
}

char_t buf[MAX_PATH];
auto size = ::GetFullPathNameW(path->c_str(), MAX_PATH, buf, nullptr);
size_t size = ::GetFullPathNameW(path->c_str(), MAX_PATH, buf, nullptr);
if (size == 0)
{
if (!skip_error_logging)
Expand All @@ -679,7 +682,7 @@ bool pal::realpath(string_t* path, bool skip_error_logging)
{
str.resize(size + LongFile::UNCExtendedPathPrefix.length(), 0);

size = ::GetFullPathNameW(path->c_str(), size, (LPWSTR)str.data(), nullptr);
size = ::GetFullPathNameW(path->c_str(), static_cast<uint32_t>(size), (LPWSTR)str.data(), nullptr);
assert(size <= str.size());

if (size == 0)
Expand Down
4 changes: 2 additions & 2 deletions src/native/corehost/hostmisc/utils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,7 @@ bool get_file_path_from_env(const pal::char_t* env_key, pal::string_t* recv)
return false;
}

size_t index_of_non_numeric(const pal::string_t& str, unsigned i)
size_t index_of_non_numeric(const pal::string_t& str, size_t i)
{
return str.find_first_not_of(_X("0123456789"), i);
}
Expand All @@ -340,7 +340,7 @@ bool try_stou(const pal::string_t& str, unsigned* num)
{
return false;
}
if (index_of_non_numeric(str, 0) != pal::string_t::npos)
if (index_of_non_numeric(str, 0u) != pal::string_t::npos)
{
return false;
}
Expand Down
11 changes: 10 additions & 1 deletion src/native/corehost/hostmisc/utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

#include "pal.h"
#include "trace.h"
#include <type_traits>

#define _STRINGIFY(s) _X(s)
#if defined(_WIN32)
Expand Down Expand Up @@ -40,7 +41,7 @@ bool get_global_shared_store_dirs(std::vector<pal::string_t>* dirs, const pal::s
bool multilevel_lookup_enabled();
void get_framework_and_sdk_locations(const pal::string_t& dotnet_dir, std::vector<pal::string_t>* locations);
bool get_file_path_from_env(const pal::char_t* env_key, pal::string_t* recv);
size_t index_of_non_numeric(const pal::string_t& str, unsigned i);
size_t index_of_non_numeric(const pal::string_t& str, size_t i);
bool try_stou(const pal::string_t& str, unsigned* num);
pal::string_t get_dotnet_root_env_var_name();
pal::string_t get_deps_from_app_binary(const pal::string_t& app_base, const pal::string_t& app);
Expand Down Expand Up @@ -115,4 +116,12 @@ class error_writer_scope_t
}
};

template<typename T>
size_t to_size_t_dbgchecked(T value)
{
assert(value >= 0);
assert(value < static_cast<T>(std::numeric_limits<size_t>::max()));
return static_cast<size_t>(value);
}

#endif
6 changes: 3 additions & 3 deletions src/native/corehost/hostpolicy/deps_resolver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ void deps_resolver_t::setup_probe_config(
m_probes.push_back(probe_config_t::published_deps_dir());

// The framework locations, starting with highest level framework.
for (size_t i = 1; i < m_fx_definitions.size(); ++i)
for (int32_t i = 1; i < static_cast<int32_t>(m_fx_definitions.size()); ++i)
{
if (pal::directory_exists(m_fx_definitions[i]->get_dir()))
{
Expand Down Expand Up @@ -572,7 +572,7 @@ bool deps_resolver_t::resolve_tpa_list(
// Probe FX deps entries after app assemblies are added.
if (m_is_framework_dependent)
{
for (size_t i = 1; i < m_fx_definitions.size(); ++i)
for (int32_t i = 1; i < static_cast<int32_t>(m_fx_definitions.size()); ++i)
{
const auto& deps_entries = m_fx_definitions[i]->get_deps().get_entries(deps_entry_t::asset_types::runtime);
for (const auto& entry : deps_entries)
Expand Down Expand Up @@ -864,7 +864,7 @@ bool deps_resolver_t::resolve_probe_dirs(
}

// Add fx package locations to fx_dir
for (size_t i = 1; i < m_fx_definitions.size(); ++i)
for (int32_t i = 1; i < static_cast<int32_t>(m_fx_definitions.size()); ++i)
{
const auto& fx_entries = m_fx_definitions[i]->get_deps().get_entries(asset_type);

Expand Down
2 changes: 1 addition & 1 deletion src/native/corehost/hostpolicy/deps_resolver.h
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ class deps_resolver_t
, m_core_servicing(args.core_servicing)
, m_is_framework_dependent(is_framework_dependent)
{
int lowest_framework = m_fx_definitions.size() - 1;
int lowest_framework = static_cast<int>(m_fx_definitions.size()) - 1;
int root_framework = -1;
if (root_framework_rid_fallback_graph == nullptr)
{
Expand Down
4 changes: 2 additions & 2 deletions src/native/corehost/hostpolicy/hostpolicy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ int run_app_for_context(
// Execute the application
unsigned int exit_code;
auto hr = context.coreclr->execute_assembly(
argv_local.size(),
(int32_t)argv_local.size(),
argv_local.data(),
managed_app.data(),
&exit_code);
Expand Down Expand Up @@ -448,7 +448,7 @@ SHARED_API int HOSTPOLICY_CALLTYPE corehost_main_with_output_buffer(const int ar
return rc;

// Get length in character count not including null terminator
int len = output_string.length();
int32_t len = static_cast<int32_t>(output_string.length());

if (len + 1 > buffer_size)
{
Expand Down
Loading

0 comments on commit 3df7a6e

Please sign in to comment.