Skip to content

Commit

Permalink
Some fixes for case insensitive paths on Windows.
Browse files Browse the repository at this point in the history
Paths on Windows are not case-sensitive.  Because of this, if a file
is called main.cpp, you should be able to set a breakpoint on it
by using the name Main.cpp.  In an ideal world, you could just
tell people to match the case, but in practice this can be a real
problem as it requires you to know whether the person who compiled
the program ran "clang++ main.cpp" or "clang++ Main.cpp", both of
which would work, regardless of what the file was actually called.

This fixes http://llvm.org/pr22667

Patch by Petr Hons

Differential Revision: http://reviews.llvm.org/D17492
Reviewed by: zturner

llvm-svn: 261771
  • Loading branch information
Zachary Turner committed Feb 24, 2016
1 parent 72c57f4 commit 47c0346
Show file tree
Hide file tree
Showing 8 changed files with 268 additions and 58 deletions.
33 changes: 31 additions & 2 deletions lldb/include/lldb/Core/ConstString.h
Expand Up @@ -290,13 +290,38 @@ class ConstString
m_string = nullptr;
}

//------------------------------------------------------------------
/// Equal to operator
///
/// Returns true if this string is equal to the string in \a rhs.
/// If case sensitive equality is tested, this operation is very
/// fast as it results in a pointer comparison since all strings
/// are in a uniqued in a global string pool.
///
/// @param[in] rhs
/// The Left Hand Side const ConstString object reference.
///
/// @param[in] rhs
/// The Right Hand Side const ConstString object reference.
///
/// @param[in] case_sensitive
/// Case sensitivity. If true, case sensitive equality
/// will be tested, otherwise character case will be ignored
///
/// @return
/// @li \b true if this object is equal to \a rhs.
/// @li \b false if this object is not equal to \a rhs.
//------------------------------------------------------------------
static bool
Equals(const ConstString &lhs, const ConstString &rhs, const bool case_sensitive = true);

//------------------------------------------------------------------
/// Compare two string objects.
///
/// Compares the C string values contained in \a lhs and \a rhs and
/// returns an integer result.
///
/// NOTE: only call this function when you want a true string
/// NOTE: only call this function when you want a true string
/// comparison. If you want string equality use the, use the ==
/// operator as it is much more efficient. Also if you want string
/// inequality, use the != operator for the same reasons.
Expand All @@ -307,13 +332,17 @@ class ConstString
/// @param[in] rhs
/// The Right Hand Side const ConstString object reference.
///
/// @param[in] case_sensitive
/// Case sensitivity of compare. If true, case sensitive compare
/// will be performed, otherwise character case will be ignored
///
/// @return
/// @li -1 if lhs < rhs
/// @li 0 if lhs == rhs
/// @li 1 if lhs > rhs
//------------------------------------------------------------------
static int
Compare (const ConstString& lhs, const ConstString& rhs);
Compare(const ConstString &lhs, const ConstString &rhs, const bool case_sensitive = true);

//------------------------------------------------------------------
/// Dump the object description to a stream.
Expand Down
13 changes: 13 additions & 0 deletions lldb/include/lldb/Host/FileSpec.h
Expand Up @@ -260,6 +260,19 @@ class FileSpec
static bool
Equal (const FileSpec& a, const FileSpec& b, bool full, bool remove_backups = false);

//------------------------------------------------------------------
/// Case sensitivity of path.
///
/// @return
/// \b true if the file path is case sensitive (POSIX), false
/// if case insensitive (Windows).
//------------------------------------------------------------------
bool
IsCaseSensitive() const
{
return m_syntax != ePathSyntaxWindows;
}

//------------------------------------------------------------------
/// Dump this object to a Stream.
///
Expand Down
@@ -0,0 +1,6 @@
LEVEL = ../../../make

C_SOURCES := main.c
CFLAGS_EXTRAS += -std=c99

include $(LEVEL)/Makefile.rules
@@ -0,0 +1,120 @@
"""
Test case sensitivity of paths on Windows / POSIX
llvm.org/pr22667
"""

import os
import lldb
from lldbsuite.test.lldbtest import *
from lldbsuite.test.decorators import *
from lldbsuite.test import lldbplatform, lldbplatformutil

class BreakpointCaseSensitivityTestCase(TestBase):
mydir = TestBase.compute_mydir(__file__)
BREAKPOINT_TEXT = 'Set a breakpoint here'

def setUp(self):
# Call super's setUp().
TestBase.setUp(self)
self.line = line_number('main.c', self.BREAKPOINT_TEXT)

@skipIf(oslist=no_match(['windows'])) # Skip for non-windows platforms
def test_breakpoint_matches_file_with_different_case(self):
"""Set breakpoint on file, should match files with different case on Windows"""
self.build()
self.case_sensitivity_breakpoint(True)

@skipIf(oslist=['windows']) # Skip for windows platforms
def test_breakpoint_doesnt_match_file_with_different_case(self):
"""Set breakpoint on file, shouldn't match files with different case on POSIX systems"""
self.build()
self.case_sensitivity_breakpoint(False)

def case_sensitivity_breakpoint(self, case_insensitive):
"""Set breakpoint on file, should match files with different case if case_insensitive is True"""

# use different case to check CreateTarget
exe = 'a.out'
if case_insensitive:
exe = exe.upper()

exe = os.path.join(os.getcwd(), exe)

# Create a target by the debugger.
self.target = self.dbg.CreateTarget(exe)
self.assertTrue(self.target, VALID_TARGET)
cwd = self.get_process_working_directory();

# try both BreakpointCreateByLocation and BreakpointCreateBySourceRegex
for regex in [False, True]:
# should always hit
self.check_breakpoint('main.c', regex, True)
# should always hit
self.check_breakpoint(os.path.join(cwd, 'main.c'), regex, True)
# different case for directory
self.check_breakpoint(os.path.join(cwd.upper(), 'main.c'),
regex,
case_insensitive)
# different case for file
self.check_breakpoint('Main.c',
regex,
case_insensitive)
# different case for both
self.check_breakpoint(os.path.join(cwd.upper(), 'Main.c'),
regex,
case_insensitive)

def check_breakpoint(self, file, source_regex, should_hit):
"""
Check breakpoint hit at given file set by given method
file:
File where insert the breakpoint
source_regex:
True for testing using BreakpointCreateBySourceRegex,
False for BreakpointCreateByLocation
should_hit:
True if the breakpoint should hit, False otherwise
"""

desc = ' file %s set by %s' % (file, 'regex' if source_regex else 'location')
if source_regex:
breakpoint = self.target.BreakpointCreateBySourceRegex(self.BREAKPOINT_TEXT,
lldb.SBFileSpec(file))
else:
breakpoint = self.target.BreakpointCreateByLocation(file, self.line)

self.assertEqual(breakpoint and breakpoint.GetNumLocations() == 1,
should_hit,
VALID_BREAKPOINT + desc)

# Get the breakpoint location from breakpoint after we verified that,
# indeed, it has one location.
location = breakpoint.GetLocationAtIndex(0)
self.assertEqual(location and location.IsEnabled(),
should_hit,
VALID_BREAKPOINT_LOCATION + desc)

process = self.target.LaunchSimple(None, None, self.get_process_working_directory())
self.assertTrue(process, PROCESS_IS_VALID + desc)

if should_hit:
# Did we hit our breakpoint?
from lldbsuite.test.lldbutil import get_threads_stopped_at_breakpoint
threads = get_threads_stopped_at_breakpoint (process, breakpoint)
self.assertEqual(len(threads), 1, "There should be a thread stopped at breakpoint" + desc)
# The hit count for the breakpoint should be 1.
self.assertEqual(breakpoint.GetHitCount(), 1)

else:
# check the breakpoint was not hit
self.assertEqual(lldb.eStateExited, process.GetState())
self.assertEqual(breakpoint.GetHitCount(), 0)

# let process finish
process.Continue()

# cleanup
self.target.BreakpointDelete(breakpoint.GetID())
@@ -0,0 +1,8 @@
#include <stdio.h>

int
main()
{
printf("Set a breakpoint here.\n");
return 0;
}
29 changes: 27 additions & 2 deletions lldb/source/Core/ConstString.cpp
Expand Up @@ -265,8 +265,25 @@ ConstString::GetLength () const
return StringPool().GetConstCStringLength (m_string);
}

bool
ConstString::Equals(const ConstString &lhs, const ConstString &rhs, const bool case_sensitive)
{
if (lhs.m_string == rhs.m_string)
return true;

// Since the pointers weren't equal, and identical ConstStrings always have identical pointers,
// the result must be false for case sensitive equality test.
if (case_sensitive)
return false;

// perform case insensitive equality test
llvm::StringRef lhs_string_ref(lhs.m_string, StringPool().GetConstCStringLength(lhs.m_string));
llvm::StringRef rhs_string_ref(rhs.m_string, StringPool().GetConstCStringLength(rhs.m_string));
return lhs_string_ref.equals_lower(rhs_string_ref);
}

int
ConstString::Compare (const ConstString& lhs, const ConstString& rhs)
ConstString::Compare(const ConstString &lhs, const ConstString &rhs, const bool case_sensitive)
{
// If the iterators are the same, this is the same string
const char *lhs_cstr = lhs.m_string;
Expand All @@ -277,7 +294,15 @@ ConstString::Compare (const ConstString& lhs, const ConstString& rhs)
{
llvm::StringRef lhs_string_ref (lhs_cstr, StringPool().GetConstCStringLength (lhs_cstr));
llvm::StringRef rhs_string_ref (rhs_cstr, StringPool().GetConstCStringLength (rhs_cstr));
return lhs_string_ref.compare(rhs_string_ref);

if (case_sensitive)
{
return lhs_string_ref.compare(rhs_string_ref);
}
else
{
return lhs_string_ref.compare_lower(rhs_string_ref);
}
}

if (lhs_cstr)
Expand Down
3 changes: 2 additions & 1 deletion lldb/source/Core/FileSpecList.cpp
Expand Up @@ -119,7 +119,8 @@ FileSpecList::FindFileIndex (size_t start_idx, const FileSpec &file_spec, bool f
{
if (compare_filename_only)
{
if (m_files[idx].GetFilename() == file_spec.GetFilename())
if (ConstString::Equals(m_files[idx].GetFilename(), file_spec.GetFilename(),
file_spec.IsCaseSensitive() || m_files[idx].IsCaseSensitive()))
return idx;
}
else
Expand Down

0 comments on commit 47c0346

Please sign in to comment.